From 636639a201bd9cfea36af53e7e4ec2c72d2d479f Mon Sep 17 00:00:00 2001 From: Thomas Kemmer Date: Wed, 6 May 2015 14:50:21 +0200 Subject: [PATCH] Fix #1162: Ignore None results and exceptions from PlaylistsProvider.create(). --- docs/changelog.rst | 9 +++++++++ mopidy/core/playlists.py | 18 ++++++++++++------ tests/core/test_playlists.py | 26 ++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 10c413ec..00cee7b8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,6 +5,15 @@ Changelog This changelog is used to track all major changes to Mopidy. +v1.0.5 (UNRELEASED) +=================== + +Bug fix release. + +- Core: Add workaround for playlist providers that do not support + creating playlists. (Fixes: :issue:`1162`, PR :issue:`1165`) + + v1.0.4 (2015-04-30) =================== diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 669e1f35..1b4c2692 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -118,13 +118,19 @@ class PlaylistsController(object): :rtype: :class:`mopidy.models.Playlist` """ if uri_scheme in self.backends.with_playlists: - backend = self.backends.with_playlists[uri_scheme] + backends = [self.backends.with_playlists[uri_scheme]] else: - # TODO: this fallback looks suspicious - backend = list(self.backends.with_playlists.values())[0] - playlist = backend.playlists.create(name).get() - listener.CoreListener.send('playlist_changed', playlist=playlist) - return playlist + backends = self.backends.with_playlists.values() + for backend in backends: + try: + playlist = backend.playlists.create(name).get() + except Exception: + playlist = None + # Workaround for playlist providers that return None from create() + if not playlist: + continue + listener.CoreListener.send('playlist_changed', playlist=playlist) + return playlist def delete(self, uri): """ diff --git a/tests/core/test_playlists.py b/tests/core/test_playlists.py index 081f73e6..e02f6204 100644 --- a/tests/core/test_playlists.py +++ b/tests/core/test_playlists.py @@ -118,6 +118,32 @@ class PlaylistsTest(unittest.TestCase): self.sp1.create.assert_called_once_with('foo') self.assertFalse(self.sp2.create.called) + def test_create_without_uri_scheme_ignores_none_result(self): + playlist = Playlist() + self.sp1.create().get.return_value = None + self.sp1.reset_mock() + self.sp2.create().get.return_value = playlist + self.sp2.reset_mock() + + result = self.core.playlists.create('foo') + + self.assertEqual(playlist, result) + self.sp1.create.assert_called_once_with('foo') + self.sp2.create.assert_called_once_with('foo') + + def test_create_without_uri_scheme_ignores_exception(self): + playlist = Playlist() + self.sp1.create().get.side_effect = Exception + self.sp1.reset_mock() + self.sp2.create().get.return_value = playlist + self.sp2.reset_mock() + + result = self.core.playlists.create('foo') + + self.assertEqual(playlist, result) + self.sp1.create.assert_called_once_with('foo') + self.sp2.create.assert_called_once_with('foo') + def test_create_with_uri_scheme_selects_the_matching_backend(self): playlist = Playlist() self.sp2.create().get.return_value = playlist