core: Define playlists.delete() return type

docs/changelog.rst

@@ -22,6 +22,9 @@ Feature release.
 - Core: Fix crash on `library.lookup(uris=[])`. (Fixes: :issue:`1619`, PR:
   :issue:`1620`)
 
+- Core: Define return value of `playlists.delete()` to be a bool, :class:`True`
+  on success, :class:`False` otherwise. (PR: :issue:`1702`)
+
 - File: Change default ordering to show directories first, then files. (PR:
   :issue:`1595`)
 

mopidy/backend.py

@@ -362,10 +362,16 @@ class PlaylistsProvider(object):
         """
         Delete playlist identified by the URI.
 
+        Returns :class:`True` if deleted, :class:`False` otherwise.
+
         *MUST be implemented by subclass.*
 
         :param uri: URI of the playlist to delete
         :type uri: string
+        :rtype: :class:`bool`
+
+        .. versionchanged:: 2.2
+            Return type defined.
         """
         raise NotImplementedError
 

mopidy/core/playlists.py

@@ -180,22 +180,35 @@ class PlaylistsController(object):
         If the URI doesn't match the URI schemes handled by the current
         backends, nothing happens.
 
+        Returns :class:`True` if deleted, :class:`False` otherwise.
+
         :param uri: URI of the playlist to delete
         :type uri: string
+        :rtype: :class:`bool`
+
+        .. versionchanged:: 2.2
+            Return type defined.
         """
         validation.check_uri(uri)
 
         uri_scheme = urllib.parse.urlparse(uri).scheme
         backend = self.backends.with_playlists.get(uri_scheme, None)
         if not backend:
-            return None  # TODO: error reporting to user
+            return False
 
+        success = False
         with _backend_error_handling(backend):
-            backend.playlists.delete(uri).get()
-            # TODO: error detection and reporting to user
+            success = backend.playlists.delete(uri).get()
+
+        if success is None:
+            # Return type was defined in Mopidy 2.2. Assume everything went
+            # well if the backend doesn't report otherwise.
+            success = True
+
+        if success:
             listener.CoreListener.send('playlist_deleted', uri=uri)
 
-        # TODO: return value?
+        return success
 
     def filter(self, criteria=None, **kwargs):
         """

mopidy/m3u/playlists.py

@@ -93,6 +93,9 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider):
             os.remove(self._abspath(path))
         except EnvironmentError as e:
             log_environment_error('Error deleting playlist %s' % uri, e)
+            return False
+        else:
+            return True
 
     def get_items(self, uri):
         path = translator.uri_to_path(uri)

tests/core/test_playlists.py

@@ -141,26 +141,30 @@ class PlaylistTest(BasePlaylistsTest):
         self.assertFalse(self.sp2.create.called)
 
     def test_delete_selects_the_dummy1_backend(self):
-        self.core.playlists.delete('dummy1:a')
+        success = self.core.playlists.delete('dummy1:a')
 
+        self.assertTrue(success)
         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.playlists.delete('dummy2:a')
+        success = self.core.playlists.delete('dummy2:a')
 
+        self.assertTrue(success)
         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.playlists.delete('unknown:a')
+        success = self.core.playlists.delete('unknown:a')
 
+        self.assertFalse(success)
         self.assertFalse(self.sp1.delete.called)
         self.assertFalse(self.sp2.delete.called)
 
     def test_delete_ignores_backend_without_playlist_support(self):
-        self.core.playlists.delete('dummy3:a')
+        success = self.core.playlists.delete('dummy3:a')
 
+        self.assertFalse(success)
         self.assertFalse(self.sp1.delete.called)
         self.assertFalse(self.sp2.delete.called)
 
@@ -377,7 +381,7 @@ class DeleteBadBackendsTest(MockBackendCorePlaylistsBase):
 
     def test_backend_raises_exception(self, logger):
         self.playlists.delete.return_value.get.side_effect = Exception
-        self.assertIsNone(self.core.playlists.delete('dummy:/1'))
+        self.assertFalse(self.core.playlists.delete('dummy:/1'))
         logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
 
 

tests/m3u/test_playlists.py

@@ -94,7 +94,8 @@ class M3UPlaylistsProviderTest(unittest.TestCase):
         self.assertEqual(uri, playlist.uri)
         self.assertTrue(os.path.exists(path))
 
-        self.core.playlists.delete(playlist.uri)
+        success = self.core.playlists.delete(playlist.uri)
+        self.assertTrue(success)
         self.assertFalse(os.path.exists(path))
 
     def test_playlist_contents_is_written_to_disk(self):