From 0ddbb4e28a64d410f312945b59f87d1764a95090 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Mon, 29 Oct 2012 20:25:58 +0100
Subject: [PATCH 01/19] Make core.stored_playlists.playlists read-only (#217)

---
 docs/changes.rst                                      | 7 +++++++
 mopidy/core/stored_playlists.py                       | 7 +------
 tests/backends/base/stored_playlists.py               | 8 +++++---
 tests/frontends/mpd/protocol/stored_playlists_test.py | 8 ++++----
 4 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/docs/changes.rst b/docs/changes.rst
index 0203a89f..7fa59a3c 100644
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -56,6 +56,13 @@ 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:
+
+  - :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.
+
 **Changes**
 
 - Made the :mod:`NAD mixer <mopidy.audio.mixers.nad>` responsive to interrupts
diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py
index 9de1545f..a3d52023 100644
--- a/mopidy/core/stored_playlists.py
+++ b/mopidy/core/stored_playlists.py
@@ -15,17 +15,12 @@ class StoredPlaylistsController(object):
         """
         Currently stored playlists.
 
-        Read/write. List of :class:`mopidy.models.Playlist`.
+        Read-only. List of :class:`mopidy.models.Playlist`.
         """
         futures = [b.stored_playlists.playlists for b in self.backends]
         results = pykka.get_all(futures)
         return list(itertools.chain(*results))
 
-    @playlists.setter  # noqa
-    def playlists(self, playlists):
-        # TODO Support multiple backends
-        self.backends[0].stored_playlists.playlists = playlists
-
     def create(self, name):
         """
         Create a new playlist.
diff --git a/tests/backends/base/stored_playlists.py b/tests/backends/base/stored_playlists.py
index 5d01996d..fca13b93 100644
--- a/tests/backends/base/stored_playlists.py
+++ b/tests/backends/base/stored_playlists.py
@@ -65,12 +65,13 @@ class StoredPlaylistsControllerTest(object):
 
     def test_get_by_name_returns_unique_match(self):
         playlist = Playlist(name='b')
-        self.stored.playlists = [Playlist(name='a'), playlist]
+        self.backend.stored_playlists.playlists = [
+            Playlist(name='a'), playlist]
         self.assertEqual(playlist, self.stored.get(name='b'))
 
     def test_get_by_name_returns_first_of_multiple_matches(self):
         playlist = Playlist(name='b')
-        self.stored.playlists = [
+        self.backend.stored_playlists.playlists = [
             playlist, Playlist(name='a'), Playlist(name='b')]
         try:
             self.stored.get(name='b')
@@ -79,7 +80,8 @@ class StoredPlaylistsControllerTest(object):
             self.assertEqual(u'"name=b" match multiple playlists', e[0])
 
     def test_get_by_name_raises_keyerror_if_no_match(self):
-        self.stored.playlists = [Playlist(name='a'), Playlist(name='b')]
+        self.backend.stored_playlists.playlists = [
+            Playlist(name='a'), Playlist(name='b')]
         try:
             self.stored.get(name='c')
             self.fail(u'Should raise LookupError if no match')
diff --git a/tests/frontends/mpd/protocol/stored_playlists_test.py b/tests/frontends/mpd/protocol/stored_playlists_test.py
index 8cfcb338..346cd37f 100644
--- a/tests/frontends/mpd/protocol/stored_playlists_test.py
+++ b/tests/frontends/mpd/protocol/stored_playlists_test.py
@@ -7,7 +7,7 @@ from tests.frontends.mpd import protocol
 
 class StoredPlaylistsHandlerTest(protocol.BaseTestCase):
     def test_listplaylist(self):
-        self.core.stored_playlists.playlists = [
+        self.backend.stored_playlists.playlists = [
             Playlist(name='name', tracks=[Track(uri='file:///dev/urandom')])]
 
         self.sendRequest(u'listplaylist "name"')
@@ -19,7 +19,7 @@ class StoredPlaylistsHandlerTest(protocol.BaseTestCase):
         self.assertEqualResponse(u'ACK [50@0] {listplaylist} No such playlist')
 
     def test_listplaylistinfo(self):
-        self.core.stored_playlists.playlists = [
+        self.backend.stored_playlists.playlists = [
             Playlist(name='name', tracks=[Track(uri='file:///dev/urandom')])]
 
         self.sendRequest(u'listplaylistinfo "name"')
@@ -35,7 +35,7 @@ class StoredPlaylistsHandlerTest(protocol.BaseTestCase):
 
     def test_listplaylists(self):
         last_modified = datetime.datetime(2001, 3, 17, 13, 41, 17, 12345)
-        self.core.stored_playlists.playlists = [
+        self.backend.stored_playlists.playlists = [
             Playlist(name='a', last_modified=last_modified)]
 
         self.sendRequest(u'listplaylists')
@@ -47,7 +47,7 @@ class StoredPlaylistsHandlerTest(protocol.BaseTestCase):
     def test_load_known_playlist_appends_to_current_playlist(self):
         self.core.current_playlist.append([Track(uri='a'), Track(uri='b')])
         self.assertEqual(len(self.core.current_playlist.tracks.get()), 2)
-        self.core.stored_playlists.playlists = [
+        self.backend.stored_playlists.playlists = [
             Playlist(name='A-list', tracks=[
                 Track(uri='c'), Track(uri='d'), Track(uri='e')])]
 

From e2474da1efa4de0425903500e3dcdec1ec3c6e9e Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 09:42:35 +0100
Subject: [PATCH 02/19] Make core.stored_playlists.create() support
 multibackend (#217)

---
 mopidy/core/stored_playlists.py     | 16 +++++++++++++---
 tests/core/stored_playlists_test.py | 22 ++++++++++++++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py
index a3d52023..55b0bdce 100644
--- a/mopidy/core/stored_playlists.py
+++ b/mopidy/core/stored_playlists.py
@@ -21,16 +21,26 @@ class StoredPlaylistsController(object):
         results = pykka.get_all(futures)
         return list(itertools.chain(*results))
 
-    def create(self, name):
+    def create(self, name, uri_scheme=None):
         """
         Create a new playlist.
 
+        If ``uri_scheme`` matches an URI scheme handled by a current backend,
+        that backend is asked to create the playlist. If ``uri_scheme`` is
+        :class:`None` or doesn't match a current backend, the first backend is
+        asked to create the playlist.
+
         :param name: name of the new playlist
         :type name: string
+        :param uri_scheme: use the backend matching the URI scheme
+        :type uri_scheme: string
         :rtype: :class:`mopidy.models.Playlist`
         """
-        # TODO Support multiple backends
-        return self.backends[0].stored_playlists.create(name).get()
+        if uri_scheme in self.backends.by_uri_scheme:
+            backend = self.backends.by_uri_scheme[uri_scheme]
+        else:
+            backend = self.backends[0]
+        return backend.stored_playlists.create(name).get()
 
     def delete(self, playlist):
         """
diff --git a/tests/core/stored_playlists_test.py b/tests/core/stored_playlists_test.py
index d92b89c0..87c90137 100644
--- a/tests/core/stored_playlists_test.py
+++ b/tests/core/stored_playlists_test.py
@@ -37,5 +37,27 @@ class StoredPlaylistsTest(unittest.TestCase):
         self.assertIn(self.pl2a, result)
         self.assertIn(self.pl2b, result)
 
+    def test_create_without_uri_scheme_uses_first_backend(self):
+        playlist = Playlist()
+        self.sp1.create().get.return_value = playlist
+        self.sp1.reset_mock()
+
+        result = self.core.stored_playlists.create('foo')
+
+        self.assertEqual(playlist, result)
+        self.sp1.create.assert_called_once_with('foo')
+        self.assertFalse(self.sp2.create.called)
+
+    def test_create_with_uri_scheme_selects_the_matching_backend(self):
+        playlist = Playlist()
+        self.sp2.create().get.return_value = playlist
+        self.sp2.reset_mock()
+
+        result = self.core.stored_playlists.create('foo', uri_scheme='dummy2')
+
+        self.assertEqual(playlist, result)
+        self.assertFalse(self.sp1.create.called)
+        self.sp2.create.assert_called_once_with('foo')
+
     # TODO The rest of the stored playlists API is pending redesign before
     # we'll update it to support multiple backends.

From 8cc1896b9df66e66970724b002ac6cbfc9cfbfc2 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 09:47:31 +0100
Subject: [PATCH 03/19] Make core.stored_playlists.lookup() support
 multibackend (#217)

---
 mopidy/core/stored_playlists.py     | 13 +++++++++----
 tests/core/stored_playlists_test.py | 12 ++++++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py
index 55b0bdce..3525ff67 100644
--- a/mopidy/core/stored_playlists.py
+++ b/mopidy/core/stored_playlists.py
@@ -1,4 +1,5 @@
 import itertools
+import urlparse
 
 import pykka
 
@@ -85,14 +86,18 @@ class StoredPlaylistsController(object):
     def lookup(self, uri):
         """
         Lookup playlist with given URI in both the set of stored playlists and
-        in any other playlist sources.
+        in any other playlist sources. Returns :class:`None` if not found.
 
         :param uri: playlist URI
         :type uri: string
-        :rtype: :class:`mopidy.models.Playlist`
+        :rtype: :class:`mopidy.models.Playlist` or :class:`None`
         """
-        # TODO Support multiple backends
-        return self.backends[0].stored_playlists.lookup(uri).get()
+        uri_scheme = urlparse.urlparse(uri).scheme
+        backend = self.backends.by_uri_scheme.get(uri_scheme, None)
+        if backend:
+            return backend.stored_playlists.lookup(uri).get()
+        else:
+            return None
 
     def refresh(self):
         """
diff --git a/tests/core/stored_playlists_test.py b/tests/core/stored_playlists_test.py
index 87c90137..aeb22e1a 100644
--- a/tests/core/stored_playlists_test.py
+++ b/tests/core/stored_playlists_test.py
@@ -59,5 +59,17 @@ class StoredPlaylistsTest(unittest.TestCase):
         self.assertFalse(self.sp1.create.called)
         self.sp2.create.assert_called_once_with('foo')
 
+    def test_lookup_selects_the_dummy1_backend(self):
+        self.core.stored_playlists.lookup('dummy1:a')
+
+        self.sp1.lookup.assert_called_once_with('dummy1:a')
+        self.assertFalse(self.sp2.lookup.called)
+
+    def test_lookup_selects_the_dummy2_backend(self):
+        self.core.stored_playlists.lookup('dummy2:a')
+
+        self.assertFalse(self.sp1.lookup.called)
+        self.sp2.lookup.assert_called_once_with('dummy2:a')
+
     # TODO The rest of the stored playlists API is pending redesign before
     # we'll update it to support multiple backends.

From fd88b974e859825fec5d59d5c73797b775fc5029 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 10:10:53 +0100
Subject: [PATCH 04/19] Make core.stored_playlists.refresh() support
 multibackend (#217)

---
 mopidy/core/stored_playlists.py     | 19 ++++++++++++++++---
 tests/core/stored_playlists_test.py | 18 ++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py
index 3525ff67..b34b8bc1 100644
--- a/mopidy/core/stored_playlists.py
+++ b/mopidy/core/stored_playlists.py
@@ -99,12 +99,25 @@ class StoredPlaylistsController(object):
         else:
             return None
 
-    def refresh(self):
+    def refresh(self, uri_scheme=None):
         """
         Refresh the stored playlists in :attr:`playlists`.
+
+        If ``uri_scheme`` is :class:`None`, all backends are asked to refresh.
+        If ``uri_scheme`` is an URI scheme handled by a backend, only that
+        backend is asked to refresh. If ``uri_scheme`` doesn't match any
+        current backend, nothing happens.
+
+        :param uri_scheme: limit to the backend matching the URI scheme
+        :type uri_scheme: string
         """
-        # TODO Support multiple backends
-        return self.backends[0].stored_playlists.refresh().get()
+        if uri_scheme is None:
+            futures = [b.stored_playlists.refresh() for b in self.backends]
+            pykka.get_all(futures)
+        else:
+            if uri_scheme in self.backends.by_uri_scheme:
+                backend = self.backends.by_uri_scheme[uri_scheme]
+                backend.stored_playlists.refresh().get()
 
     def rename(self, playlist, new_name):
         """
diff --git a/tests/core/stored_playlists_test.py b/tests/core/stored_playlists_test.py
index aeb22e1a..2e90416e 100644
--- a/tests/core/stored_playlists_test.py
+++ b/tests/core/stored_playlists_test.py
@@ -71,5 +71,23 @@ class StoredPlaylistsTest(unittest.TestCase):
         self.assertFalse(self.sp1.lookup.called)
         self.sp2.lookup.assert_called_once_with('dummy2:a')
 
+    def test_refresh_without_uri_scheme_refreshes_all_backends(self):
+        self.core.stored_playlists.refresh()
+
+        self.sp1.refresh.assert_called_once_with()
+        self.sp2.refresh.assert_called_once_with()
+
+    def test_refresh_with_uri_scheme_refreshes_matching_backend(self):
+        self.core.stored_playlists.refresh(uri_scheme='dummy2')
+
+        self.assertFalse(self.sp1.refresh.called)
+        self.sp2.refresh.assert_called_once_with()
+
+    def test_refresh_with_unknown_uri_scheme_refreshes_nothing(self):
+        self.core.stored_playlists.refresh(uri_scheme='foobar')
+
+        self.assertFalse(self.sp1.refresh.called)
+        self.assertFalse(self.sp2.refresh.called)
+
     # TODO The rest of the stored playlists API is pending redesign before
     # we'll update it to support multiple backends.

From d8378e9284f8c42d31c29310ff1466c098e93d74 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 11:12:07 +0100
Subject: [PATCH 05/19] Set URI on local playlists when reading from disk
 (#217)

---
 mopidy/backends/local/stored_playlists.py     | 8 +++++---
 tests/backends/local/stored_playlists_test.py | 5 +++++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py
index 921fa40c..49d6edba 100644
--- a/mopidy/backends/local/stored_playlists.py
+++ b/mopidy/backends/local/stored_playlists.py
@@ -6,6 +6,7 @@ import shutil
 from mopidy import settings
 from mopidy.backends import base
 from mopidy.models import Playlist
+from mopidy.utils import path
 
 from .translator import parse_m3u
 
@@ -27,14 +28,15 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         logger.info('Loading playlists from %s', self._folder)
 
         for m3u in glob.glob(os.path.join(self._folder, '*.m3u')):
+            uri = path.path_to_uri(m3u)
             name = os.path.splitext(os.path.basename(m3u))[0]
             tracks = []
-            for uri in parse_m3u(m3u, settings.LOCAL_MUSIC_PATH):
+            for track_uri in parse_m3u(m3u, settings.LOCAL_MUSIC_PATH):
                 try:
-                    tracks.append(self.backend.library.lookup(uri))
+                    tracks.append(self.backend.library.lookup(track_uri))
                 except LookupError as ex:
                     logger.error('Playlist item could not be added: %s', ex)
-            playlist = Playlist(tracks=tracks, name=name)
+            playlist = Playlist(uri=uri, name=name, tracks=tracks)
 
             # FIXME playlist name needs better handling
             # FIXME tracks should come from lib. lookup
diff --git a/tests/backends/local/stored_playlists_test.py b/tests/backends/local/stored_playlists_test.py
index 188eb589..d1d6989a 100644
--- a/tests/backends/local/stored_playlists_test.py
+++ b/tests/backends/local/stored_playlists_test.py
@@ -57,6 +57,8 @@ class LocalStoredPlaylistsControllerTest(
         self.assertEqual(uri, contents.strip())
 
     def test_playlists_are_loaded_at_startup(self):
+        playlist_path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
+
         track = Track(uri=path_to_uri(path_to_data_dir('uri2')))
         playlist = self.stored.create('test')
         playlist = playlist.copy(tracks=[track])
@@ -65,6 +67,9 @@ class LocalStoredPlaylistsControllerTest(
         backend = self.backend_class(audio=self.audio)
 
         self.assert_(backend.stored_playlists.playlists)
+        self.assertEqual(
+            path_to_uri(playlist_path),
+            backend.stored_playlists.playlists[0].uri)
         self.assertEqual(
             playlist.name, backend.stored_playlists.playlists[0].name)
         self.assertEqual(

From 51aab4f138ab019e892c16ffafacdb732278a29d Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 11:39:13 +0100
Subject: [PATCH 06/19] Make local stored playlists set and use URIs (#217)

---
 mopidy/backends/local/stored_playlists.py     | 14 ++++-
 tests/backends/base/stored_playlists.py       | 30 ++++++----
 tests/backends/local/stored_playlists_test.py | 55 +++++++++----------
 3 files changed, 57 insertions(+), 42 deletions(-)

diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py
index 49d6edba..ef7cc92d 100644
--- a/mopidy/backends/local/stored_playlists.py
+++ b/mopidy/backends/local/stored_playlists.py
@@ -20,7 +20,9 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         self.refresh()
 
     def lookup(self, uri):
-        pass  # TODO
+        for playlist in self._playlists:
+            if playlist.uri == uri:
+                return playlist
 
     def refresh(self):
         playlists = []
@@ -46,7 +48,9 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         self.playlists = playlists
 
     def create(self, name):
-        playlist = Playlist(name=name)
+        file_path = os.path.join(self._folder, name + '.m3u')
+        uri = path.path_to_uri(file_path)
+        playlist = Playlist(uri=uri, name=name)
         self.save(playlist)
         return playlist
 
@@ -74,9 +78,10 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         shutil.move(src, dst)
 
     def save(self, playlist):
-        file_path = os.path.join(self._folder, playlist.name + '.m3u')
+        assert playlist.uri, 'Cannot save playlist without URI'
 
         # FIXME this should be a save_m3u function, not inside save
+        file_path = playlist.uri[len('file://'):]
         with open(file_path, 'w') as file_handle:
             for track in playlist.tracks:
                 if track.uri.startswith('file://'):
@@ -84,4 +89,7 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
                 else:
                     file_handle.write(track.uri + '\n')
 
+        original_playlist = self.lookup(playlist.uri)
+        if original_playlist is not None:
+            self._playlists.remove(original_playlist)
         self._playlists.append(playlist)
diff --git a/tests/backends/base/stored_playlists.py b/tests/backends/base/stored_playlists.py
index fca13b93..209aad0a 100644
--- a/tests/backends/base/stored_playlists.py
+++ b/tests/backends/base/stored_playlists.py
@@ -30,11 +30,15 @@ class StoredPlaylistsControllerTest(object):
 
         settings.runtime.clear()
 
-    def test_create(self):
+    def test_create_returns_playlist_with_name_set(self):
         playlist = self.stored.create('test')
         self.assertEqual(playlist.name, 'test')
 
-    def test_create_in_playlists(self):
+    def test_create_returns_playlist_with_uri_set(self):
+        playlist = self.stored.create('test')
+        self.assert_(playlist.uri)
+
+    def test_create_adds_playlist_to_playlists_collection(self):
         playlist = self.stored.create('test')
         self.assert_(self.stored.playlists)
         self.assertIn(playlist, self.stored.playlists)
@@ -88,9 +92,12 @@ class StoredPlaylistsControllerTest(object):
         except LookupError as e:
             self.assertEqual(u'"name=c" match no playlists', e[0])
 
-    @unittest.SkipTest
-    def test_lookup(self):
-        pass
+    def test_lookup_finds_playlist_by_uri(self):
+        original_playlist = self.stored.create('test')
+
+        looked_up_playlist = self.stored.lookup(original_playlist.uri)
+
+        self.assertEqual(original_playlist, looked_up_playlist)
 
     @unittest.SkipTest
     def test_refresh(self):
@@ -106,11 +113,14 @@ class StoredPlaylistsControllerTest(object):
         test = lambda: self.stored.get(name='test2')
         self.assertRaises(LookupError, test)
 
-    def test_save(self):
-        # FIXME should we handle playlists without names?
-        playlist = Playlist(name='test')
-        self.stored.save(playlist)
-        self.assertIn(playlist, self.stored.playlists)
+    def test_save_replaces_playlist_with_updated_playlist(self):
+        playlist1 = self.stored.create('test1')
+        self.assertIn(playlist1, self.stored.playlists)
+
+        playlist2 = playlist1.copy(name='test2')
+        self.stored.save(playlist2)
+        self.assertNotIn(playlist1, self.stored.playlists)
+        self.assertIn(playlist2, self.stored.playlists)
 
     @unittest.SkipTest
     def test_playlist_with_unknown_track(self):
diff --git a/tests/backends/local/stored_playlists_test.py b/tests/backends/local/stored_playlists_test.py
index d1d6989a..446d87f1 100644
--- a/tests/backends/local/stored_playlists_test.py
+++ b/tests/backends/local/stored_playlists_test.py
@@ -2,7 +2,7 @@ import os
 
 from mopidy import settings
 from mopidy.backends.local import LocalBackend
-from mopidy.models import Playlist, Track
+from mopidy.models import Track
 from mopidy.utils.path import path_to_uri
 
 from tests import unittest, path_to_data_dir
@@ -23,38 +23,43 @@ class LocalStoredPlaylistsControllerTest(
         self.assert_(os.path.exists(path))
 
     def test_saved_playlist_is_persisted(self):
-        path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2.m3u')
-        self.assert_(not os.path.exists(path))
-        self.stored.save(Playlist(name='test2'))
-        self.assert_(os.path.exists(path))
+        path1 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
+        path2 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2.m3u')
+
+        playlist = self.stored.create('test')
+
+        self.assertFalse(os.path.exists(path2))
+
+        self.stored.rename(playlist, 'test2')
+
+        self.assertFalse(os.path.exists(path1))
+        self.assertTrue(os.path.exists(path2))
 
     def test_deleted_playlist_is_removed(self):
-        playlist = self.stored.create('test')
-        self.stored.delete(playlist)
         path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
-        self.assert_(not os.path.exists(path))
 
-    def test_renamed_playlist_is_moved(self):
+        self.assertFalse(os.path.exists(path))
+
         playlist = self.stored.create('test')
-        file1 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
-        file2 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2.m3u')
-        self.assert_(not os.path.exists(file2))
-        self.stored.rename(playlist, 'test2')
-        self.assert_(not os.path.exists(file1))
-        self.assert_(os.path.exists(file2))
+
+        self.assertTrue(os.path.exists(path))
+
+        self.stored.delete(playlist)
+
+        self.assertFalse(os.path.exists(path))
 
     def test_playlist_contents_is_written_to_disk(self):
         track = Track(uri=generate_song(1))
-        uri = track.uri[len('file://'):]
-        playlist = Playlist(tracks=[track], name='test')
-        path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
-
+        track_path = track.uri[len('file://'):]
+        playlist = self.stored.create('test')
+        playlist_path = playlist.uri[len('file://'):]
+        playlist = playlist.copy(tracks=[track])
         self.stored.save(playlist)
 
-        with open(path) as playlist_file:
+        with open(playlist_path) as playlist_file:
             contents = playlist_file.read()
 
-        self.assertEqual(uri, contents.strip())
+        self.assertEqual(track_path, contents.strip())
 
     def test_playlists_are_loaded_at_startup(self):
         playlist_path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
@@ -82,11 +87,3 @@ class LocalStoredPlaylistsControllerTest(
     @unittest.SkipTest
     def test_playlist_folder_is_createad(self):
         pass
-
-    @unittest.SkipTest
-    def test_create_sets_playlist_uri(self):
-        pass
-
-    @unittest.SkipTest
-    def test_save_sets_playlist_uri(self):
-        pass

From d03881f173e78cf02abc4037f1b5634d24cee281 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 15:19:04 +0100
Subject: [PATCH 07/19] Require stored_playlists.save() to return the updated
 playlist (#217)

---
 docs/changes.rst                | 4 ++++
 mopidy/core/stored_playlists.py | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/docs/changes.rst b/docs/changes.rst
index 7fa59a3c..4d58ff4e 100644
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -63,6 +63,10 @@ backends:
     and all functionality is maintained by assigning to the playlists
     collections at the backend level.
 
+  - :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 saved playlist.
+
 **Changes**
 
 - Made the :mod:`NAD mixer <mopidy.audio.mixers.nad>` responsive to interrupts
diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py
index b34b8bc1..8f87db58 100644
--- a/mopidy/core/stored_playlists.py
+++ b/mopidy/core/stored_playlists.py
@@ -136,8 +136,15 @@ class StoredPlaylistsController(object):
         """
         Save the playlist to the set of stored playlists.
 
+        Returns the saved playlist. The return playlist may differ from the
+        saved playlist. E.g. if the playlist name was changed, the returned
+        playlist may have a different URI. The caller of this method should
+        throw away the playlist sent to this method, and use the returned
+        playlist instead.
+
         :param playlist: the playlist
         :type playlist: :class:`mopidy.models.Playlist`
+        :rtype: :class:`mopidy.models.Playlist`
         """
         # TODO Support multiple backends
         return self.backends[0].stored_playlists.save(playlist).get()

From 06bcad2db9554c0a7c0bf8217dd3c1451fa2f243 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 15:21:02 +0100
Subject: [PATCH 08/19] Make local.stored_playlists.save() capable of renaming
 playlists (#217)

---
 docs/changes.rst                              |  2 +-
 mopidy/backends/local/stored_playlists.py     | 70 +++++++++++--------
 tests/backends/base/stored_playlists.py       |  4 +-
 tests/backends/local/stored_playlists_test.py | 12 ++--
 4 files changed, 52 insertions(+), 36 deletions(-)

diff --git a/docs/changes.rst b/docs/changes.rst
index 4d58ff4e..285af6b3 100644
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -65,7 +65,7 @@ backends:
 
   - :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 saved playlist.
+    should thus be used instead of the playlist passed to ``save()``.
 
 **Changes**
 
diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py
index ef7cc92d..621d37bf 100644
--- a/mopidy/backends/local/stored_playlists.py
+++ b/mopidy/backends/local/stored_playlists.py
@@ -10,6 +10,7 @@ from mopidy.utils import path
 
 from .translator import parse_m3u
 
+
 logger = logging.getLogger(u'mopidy.backends.local')
 
 
@@ -19,6 +20,25 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         self._folder = settings.LOCAL_PLAYLIST_PATH
         self.refresh()
 
+    # TODO playlist names needs safer handling using a slug function
+
+    def create(self, name):
+        file_path = os.path.join(self._folder, name + '.m3u')
+        uri = path.path_to_uri(file_path)
+        playlist = Playlist(uri=uri, name=name)
+        self.save(playlist)
+        return playlist
+
+    def delete(self, playlist):
+        if playlist not in self._playlists:
+            return
+
+        self._playlists.remove(playlist)
+        filename = os.path.join(self._folder, playlist.name + '.m3u')
+
+        if os.path.exists(filename):
+            os.remove(filename)
+
     def lookup(self, uri):
         for playlist in self._playlists:
             if playlist.uri == uri:
@@ -40,30 +60,10 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
                     logger.error('Playlist item could not be added: %s', ex)
             playlist = Playlist(uri=uri, name=name, tracks=tracks)
 
-            # FIXME playlist name needs better handling
-            # FIXME tracks should come from lib. lookup
-
             playlists.append(playlist)
 
         self.playlists = playlists
 
-    def create(self, name):
-        file_path = os.path.join(self._folder, name + '.m3u')
-        uri = path.path_to_uri(file_path)
-        playlist = Playlist(uri=uri, name=name)
-        self.save(playlist)
-        return playlist
-
-    def delete(self, playlist):
-        if playlist not in self._playlists:
-            return
-
-        self._playlists.remove(playlist)
-        filename = os.path.join(self._folder, playlist.name + '.m3u')
-
-        if os.path.exists(filename):
-            os.remove(filename)
-
     def rename(self, playlist, name):
         if playlist not in self._playlists:
             return
@@ -80,16 +80,30 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
     def save(self, playlist):
         assert playlist.uri, 'Cannot save playlist without URI'
 
-        # FIXME this should be a save_m3u function, not inside save
+        old_playlist = self.lookup(playlist.uri)
+
+        if old_playlist and playlist.name != old_playlist.name:
+            src = os.path.join(self._folder, old_playlist.name + '.m3u')
+            dst = os.path.join(self._folder, playlist.name + '.m3u')
+            shutil.move(src, dst)
+            playlist = playlist.copy(uri=path.path_to_uri(dst))
+
+        self._save_m3u(playlist)
+
+        if old_playlist is not None:
+            index = self._playlists.index(old_playlist)
+            self._playlists[index] = playlist
+        else:
+            self._playlists.append(playlist)
+
+        return playlist
+
+    def _save_m3u(self, playlist):
         file_path = playlist.uri[len('file://'):]
         with open(file_path, 'w') as file_handle:
             for track in playlist.tracks:
                 if track.uri.startswith('file://'):
-                    file_handle.write(track.uri[len('file://'):] + '\n')
+                    uri = track.uri[len('file://'):]
                 else:
-                    file_handle.write(track.uri + '\n')
-
-        original_playlist = self.lookup(playlist.uri)
-        if original_playlist is not None:
-            self._playlists.remove(original_playlist)
-        self._playlists.append(playlist)
+                    uri = track.uri
+                file_handle.write(uri + '\n')
diff --git a/tests/backends/base/stored_playlists.py b/tests/backends/base/stored_playlists.py
index 209aad0a..83c243f3 100644
--- a/tests/backends/base/stored_playlists.py
+++ b/tests/backends/base/stored_playlists.py
@@ -113,12 +113,12 @@ class StoredPlaylistsControllerTest(object):
         test = lambda: self.stored.get(name='test2')
         self.assertRaises(LookupError, test)
 
-    def test_save_replaces_playlist_with_updated_playlist(self):
+    def test_save_replaces_stored_playlist_with_updated_playlist(self):
         playlist1 = self.stored.create('test1')
         self.assertIn(playlist1, self.stored.playlists)
 
         playlist2 = playlist1.copy(name='test2')
-        self.stored.save(playlist2)
+        playlist2 = self.stored.save(playlist2)
         self.assertNotIn(playlist1, self.stored.playlists)
         self.assertIn(playlist2, self.stored.playlists)
 
diff --git a/tests/backends/local/stored_playlists_test.py b/tests/backends/local/stored_playlists_test.py
index 446d87f1..7025c402 100644
--- a/tests/backends/local/stored_playlists_test.py
+++ b/tests/backends/local/stored_playlists_test.py
@@ -23,14 +23,16 @@ class LocalStoredPlaylistsControllerTest(
         self.assert_(os.path.exists(path))
 
     def test_saved_playlist_is_persisted(self):
-        path1 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
+        path1 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test1.m3u')
         path2 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2.m3u')
 
-        playlist = self.stored.create('test')
+        playlist = self.stored.create('test1')
 
+        self.assertTrue(os.path.exists(path1))
         self.assertFalse(os.path.exists(path2))
 
-        self.stored.rename(playlist, 'test2')
+        playlist = playlist.copy(name='test2')
+        playlist = self.stored.save(playlist)
 
         self.assertFalse(os.path.exists(path1))
         self.assertTrue(os.path.exists(path2))
@@ -54,7 +56,7 @@ class LocalStoredPlaylistsControllerTest(
         playlist = self.stored.create('test')
         playlist_path = playlist.uri[len('file://'):]
         playlist = playlist.copy(tracks=[track])
-        self.stored.save(playlist)
+        playlist = self.stored.save(playlist)
 
         with open(playlist_path) as playlist_file:
             contents = playlist_file.read()
@@ -67,7 +69,7 @@ class LocalStoredPlaylistsControllerTest(
         track = Track(uri=path_to_uri(path_to_data_dir('uri2')))
         playlist = self.stored.create('test')
         playlist = playlist.copy(tracks=[track])
-        self.stored.save(playlist)
+        playlist = self.stored.save(playlist)
 
         backend = self.backend_class(audio=self.audio)
 

From f9f6f9394dbdeec8a858e9ab82089d16a6893a98 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 15:25:39 +0100
Subject: [PATCH 09/19] Remove stored_playlists.rename() (#217)

---
 docs/changes.rst                          |  3 +++
 mopidy/backends/base.py                   |  8 --------
 mopidy/backends/local/stored_playlists.py | 13 -------------
 mopidy/core/stored_playlists.py           | 13 -------------
 tests/backends/base/stored_playlists.py   | 10 ----------
 5 files changed, 3 insertions(+), 44 deletions(-)

diff --git a/docs/changes.rst b/docs/changes.rst
index 285af6b3..9c2bea62 100644
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -67,6 +67,9 @@ backends:
     playlist. The returned playlist may differ from the saved playlist, and
     should thus be used instead of the playlist passed to ``save()``.
 
+  - :meth:`mopidy.core.StoredPlaylistsController.rename` has been removed,
+    since renaming can be done with ``save()``.
+
 **Changes**
 
 - Made the :mod:`NAD mixer <mopidy.audio.mixers.nad>` responsive to interrupts
diff --git a/mopidy/backends/base.py b/mopidy/backends/base.py
index 7ae2c3dc..e4c40a92 100644
--- a/mopidy/backends/base.py
+++ b/mopidy/backends/base.py
@@ -204,14 +204,6 @@ class BaseStoredPlaylistsProvider(object):
         """
         raise NotImplementedError
 
-    def rename(self, playlist, new_name):
-        """
-        See :meth:`mopidy.core.StoredPlaylistsController.rename`.
-
-        *MUST be implemented by subclass.*
-        """
-        raise NotImplementedError
-
     def save(self, playlist):
         """
         See :meth:`mopidy.core.StoredPlaylistsController.save`.
diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py
index 621d37bf..6d12dd46 100644
--- a/mopidy/backends/local/stored_playlists.py
+++ b/mopidy/backends/local/stored_playlists.py
@@ -64,19 +64,6 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
 
         self.playlists = playlists
 
-    def rename(self, playlist, name):
-        if playlist not in self._playlists:
-            return
-
-        src = os.path.join(self._folder, playlist.name + '.m3u')
-        dst = os.path.join(self._folder, name + '.m3u')
-
-        renamed = playlist.copy(name=name)
-        index = self._playlists.index(playlist)
-        self._playlists[index] = renamed
-
-        shutil.move(src, dst)
-
     def save(self, playlist):
         assert playlist.uri, 'Cannot save playlist without URI'
 
diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py
index 8f87db58..af88d86b 100644
--- a/mopidy/core/stored_playlists.py
+++ b/mopidy/core/stored_playlists.py
@@ -119,19 +119,6 @@ class StoredPlaylistsController(object):
                 backend = self.backends.by_uri_scheme[uri_scheme]
                 backend.stored_playlists.refresh().get()
 
-    def rename(self, playlist, new_name):
-        """
-        Rename playlist.
-
-        :param playlist: the playlist
-        :type playlist: :class:`mopidy.models.Playlist`
-        :param new_name: the new name
-        :type new_name: string
-        """
-        # TODO Support multiple backends
-        return self.backends[0].stored_playlists.rename(
-            playlist, new_name).get()
-
     def save(self, playlist):
         """
         Save the playlist to the set of stored playlists.
diff --git a/tests/backends/base/stored_playlists.py b/tests/backends/base/stored_playlists.py
index 83c243f3..ba907b13 100644
--- a/tests/backends/base/stored_playlists.py
+++ b/tests/backends/base/stored_playlists.py
@@ -103,16 +103,6 @@ class StoredPlaylistsControllerTest(object):
     def test_refresh(self):
         pass
 
-    def test_rename(self):
-        playlist = self.stored.create('test')
-        self.stored.rename(playlist, 'test2')
-        self.stored.get(name='test2')
-
-    def test_rename_unknown_playlist(self):
-        self.stored.rename(Playlist(), 'test2')
-        test = lambda: self.stored.get(name='test2')
-        self.assertRaises(LookupError, test)
-
     def test_save_replaces_stored_playlist_with_updated_playlist(self):
         playlist1 = self.stored.create('test1')
         self.assertIn(playlist1, self.stored.playlists)

From 3d05f3c65f26e04fda43e57406ab761de2b896f3 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 16:37:07 +0100
Subject: [PATCH 10/19] 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')
 

From 6c49a7fc525599fc2a4f5cdb2c7b4c8905751b2b Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Wed, 31 Oct 2012 16:52:34 +0100
Subject: [PATCH 11/19] Make core.stored_playlists.save() support multibackend
 (#217)

---
 mopidy/core/stored_playlists.py     | 32 ++++++++++++++++++-------
 tests/core/stored_playlists_test.py | 37 +++++++++++++++++++++++++++--
 2 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py
index 16dffdb0..c404a408 100644
--- a/mopidy/core/stored_playlists.py
+++ b/mopidy/core/stored_playlists.py
@@ -31,6 +31,9 @@ class StoredPlaylistsController(object):
         :class:`None` or doesn't match a current backend, the first backend is
         asked to create the playlist.
 
+        All new playlists should be created by calling this method, and **not**
+        by creating new instances of :class:`mopidy.models.Playlist`.
+
         :param name: name of the new playlist
         :type name: string
         :param uri_scheme: use the backend matching the URI scheme
@@ -128,15 +131,28 @@ class StoredPlaylistsController(object):
         """
         Save the playlist to the set of stored playlists.
 
-        Returns the saved playlist. The return playlist may differ from the
-        saved playlist. E.g. if the playlist name was changed, the returned
-        playlist may have a different URI. The caller of this method should
-        throw away the playlist sent to this method, and use the returned
-        playlist instead.
+        For a playlist to be saveable, it must have the ``uri`` attribute set.
+        You should not set the ``uri`` atribute yourself, but use playlist
+        objects returned by :meth:`create` or retrieved from :attr:`playlists`,
+        which will always give you saveable playlists.
+
+        The method returns the saved playlist. The return playlist may differ
+        from the saved playlist. E.g. if the playlist name was changed, the
+        returned playlist may have a different URI. The caller of this method
+        should throw away the playlist sent to this method, and use the
+        returned playlist instead.
+
+        If the playlist's URI isn't set or doesn't match the URI scheme of a
+        current backend, nothing is done and :class:`None` is returned.
 
         :param playlist: the playlist
         :type playlist: :class:`mopidy.models.Playlist`
-        :rtype: :class:`mopidy.models.Playlist`
+        :rtype: :class:`mopidy.models.Playlist` or :class:`None`
         """
-        # TODO Support multiple backends
-        return self.backends[0].stored_playlists.save(playlist).get()
+        if playlist.uri is None:
+            return
+        uri_scheme = urlparse.urlparse(playlist.uri).scheme
+        if uri_scheme not in self.backends.by_uri_scheme:
+            return
+        backend = self.backends.by_uri_scheme[uri_scheme]
+        return backend.stored_playlists.save(playlist).get()
diff --git a/tests/core/stored_playlists_test.py b/tests/core/stored_playlists_test.py
index 39914766..b0d48512 100644
--- a/tests/core/stored_playlists_test.py
+++ b/tests/core/stored_playlists_test.py
@@ -107,5 +107,38 @@ class StoredPlaylistsTest(unittest.TestCase):
         self.assertFalse(self.sp1.refresh.called)
         self.assertFalse(self.sp2.refresh.called)
 
-    # TODO The rest of the stored playlists API is pending redesign before
-    # we'll update it to support multiple backends.
+    def test_save_selects_the_dummy1_backend(self):
+        playlist = Playlist(uri='dummy1:a')
+        self.sp1.save().get.return_value = playlist
+        self.sp1.reset_mock()
+
+        result = self.core.stored_playlists.save(playlist)
+
+        self.assertEqual(playlist, result)
+        self.sp1.save.assert_called_once_with(playlist)
+        self.assertFalse(self.sp2.save.called)
+
+    def test_save_selects_the_dummy2_backend(self):
+        playlist = Playlist(uri='dummy2:a')
+        self.sp2.save().get.return_value = playlist
+        self.sp2.reset_mock()
+
+        result = self.core.stored_playlists.save(playlist)
+
+        self.assertEqual(playlist, result)
+        self.assertFalse(self.sp1.save.called)
+        self.sp2.save.assert_called_once_with(playlist)
+
+    def test_save_does_nothing_if_playlist_uri_is_unset(self):
+        result = self.core.stored_playlists.save(Playlist())
+
+        self.assertIsNone(result)
+        self.assertFalse(self.sp1.save.called)
+        self.assertFalse(self.sp2.save.called)
+
+    def test_save_does_nothing_if_playlist_uri_has_unknown_scheme(self):
+        result = self.core.stored_playlists.save(Playlist(uri='foobar:a'))
+
+        self.assertIsNone(result)
+        self.assertFalse(self.sp1.save.called)
+        self.assertFalse(self.sp2.save.called)

From 58c190f12b925034182e4e219824f5dc8cf00005 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Thu, 1 Nov 2012 12:10:17 +0100
Subject: [PATCH 12/19] Fix grammar (#217)

---
 docs/changes.rst | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/docs/changes.rst b/docs/changes.rst
index 1427c867..ef0d3bcd 100644
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -56,9 +56,8 @@ 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 to be more
-  focused around the playlist URI, and some redundant functionality have been
-  removed:
+- The stored playlists part of the core API has been revised to be more focused
+  around the playlist URI, and some redundant functionality has been removed:
 
   - :attr:`mopidy.core.StoredPlaylistsController.playlists` no longer supports
     assignment to it. The `playlists` property on the backend layer still does,

From 078cc72fffbf72a30f2f30969d7dfe033c5ac8fb Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Thu, 1 Nov 2012 12:12:01 +0100
Subject: [PATCH 13/19] Remove undocumented return from
 core.stored_playlists.delete() (#217)

---
 mopidy/core/stored_playlists.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py
index c404a408..8c04d5ad 100644
--- a/mopidy/core/stored_playlists.py
+++ b/mopidy/core/stored_playlists.py
@@ -57,9 +57,9 @@ class StoredPlaylistsController(object):
         :type uri: string
         """
         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()
+        if uri_scheme in self.backends.by_uri_scheme:
+            backend = self.backends.by_uri_scheme[uri_scheme]
+            backend.stored_playlists.delete(uri).get()
 
     def get(self, **criteria):
         """

From 8c9a3d6df232faf1f61f2aeb6cf827f506e50743 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Thu, 1 Nov 2012 12:46:29 +0100
Subject: [PATCH 14/19] Slugify local playlist names to make them safe to use
 in paths (#217)

---
 mopidy/backends/local/stored_playlists.py     | 23 +++++++++---
 tests/backends/base/stored_playlists.py       | 16 ++++-----
 tests/backends/local/stored_playlists_test.py | 36 ++++++++++++++-----
 3 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py
index 139cfac8..3d488655 100644
--- a/mopidy/backends/local/stored_playlists.py
+++ b/mopidy/backends/local/stored_playlists.py
@@ -1,7 +1,9 @@
 import glob
 import logging
 import os
+import re
 import shutil
+import unicodedata
 
 from mopidy import settings
 from mopidy.backends import base
@@ -20,9 +22,8 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         self._folder = settings.LOCAL_PLAYLIST_PATH
         self.refresh()
 
-    # TODO playlist names needs safer handling using a slug function
-
     def create(self, name):
+        name = self._slugify(name)
         file_path = os.path.join(self._folder, name + '.m3u')
         uri = path.path_to_uri(file_path)
         playlist = Playlist(uri=uri, name=name)
@@ -68,10 +69,11 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         old_playlist = self.lookup(playlist.uri)
 
         if old_playlist and playlist.name != old_playlist.name:
+            new_name = self._slugify(playlist.name)
             src = os.path.join(self._folder, old_playlist.name + '.m3u')
-            dst = os.path.join(self._folder, playlist.name + '.m3u')
+            dst = os.path.join(self._folder, new_name + '.m3u')
             shutil.move(src, dst)
-            playlist = playlist.copy(uri=path.path_to_uri(dst))
+            playlist = playlist.copy(uri=path.path_to_uri(dst), name=new_name)
 
         self._save_m3u(playlist)
 
@@ -97,3 +99,16 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         file_path = playlist.uri[len('file://'):]
         if os.path.exists(file_path):
             os.remove(file_path)
+
+    def _slugify(self, value):
+        """
+        Converts to lowercase, removes non-word characters (alphanumerics and
+        underscores) and converts spaces to hyphens. Also strips leading and
+        trailing whitespace.
+
+        This function is based on Django's slugify implementation.
+        """
+        value = unicodedata.normalize('NFKD', value)
+        value = value.encode('ascii', 'ignore').decode('ascii')
+        value = re.sub('[^\w\s-]', '', value).strip().lower()
+        return re.sub('[-\s]+', '-', value)
diff --git a/tests/backends/base/stored_playlists.py b/tests/backends/base/stored_playlists.py
index 2b5469ac..267a025c 100644
--- a/tests/backends/base/stored_playlists.py
+++ b/tests/backends/base/stored_playlists.py
@@ -31,15 +31,15 @@ class StoredPlaylistsControllerTest(object):
         settings.runtime.clear()
 
     def test_create_returns_playlist_with_name_set(self):
-        playlist = self.stored.create('test')
+        playlist = self.stored.create(u'test')
         self.assertEqual(playlist.name, 'test')
 
     def test_create_returns_playlist_with_uri_set(self):
-        playlist = self.stored.create('test')
+        playlist = self.stored.create(u'test')
         self.assert_(playlist.uri)
 
     def test_create_adds_playlist_to_playlists_collection(self):
-        playlist = self.stored.create('test')
+        playlist = self.stored.create(u'test')
         self.assert_(self.stored.playlists)
         self.assertIn(playlist, self.stored.playlists)
 
@@ -50,7 +50,7 @@ class StoredPlaylistsControllerTest(object):
         self.stored.delete('file:///unknown/playlist')
 
     def test_delete_playlist_removes_it_from_the_collection(self):
-        playlist = self.stored.create('test')
+        playlist = self.stored.create(u'test')
         self.assertIn(playlist, self.stored.playlists)
 
         self.stored.delete(playlist.uri)
@@ -66,7 +66,7 @@ class StoredPlaylistsControllerTest(object):
         self.assertRaises(LookupError, test)
 
     def test_get_with_right_criteria(self):
-        playlist1 = self.stored.create('test')
+        playlist1 = self.stored.create(u'test')
         playlist2 = self.stored.get(name='test')
         self.assertEqual(playlist1, playlist2)
 
@@ -96,7 +96,7 @@ class StoredPlaylistsControllerTest(object):
             self.assertEqual(u'"name=c" match no playlists', e[0])
 
     def test_lookup_finds_playlist_by_uri(self):
-        original_playlist = self.stored.create('test')
+        original_playlist = self.stored.create(u'test')
 
         looked_up_playlist = self.stored.lookup(original_playlist.uri)
 
@@ -107,10 +107,10 @@ class StoredPlaylistsControllerTest(object):
         pass
 
     def test_save_replaces_stored_playlist_with_updated_playlist(self):
-        playlist1 = self.stored.create('test1')
+        playlist1 = self.stored.create(u'test1')
         self.assertIn(playlist1, self.stored.playlists)
 
-        playlist2 = playlist1.copy(name='test2')
+        playlist2 = playlist1.copy(name=u'test2')
         playlist2 = self.stored.save(playlist2)
         self.assertNotIn(playlist1, self.stored.playlists)
         self.assertIn(playlist2, self.stored.playlists)
diff --git a/tests/backends/local/stored_playlists_test.py b/tests/backends/local/stored_playlists_test.py
index 987c0788..cd1ecd3c 100644
--- a/tests/backends/local/stored_playlists_test.py
+++ b/tests/backends/local/stored_playlists_test.py
@@ -18,22 +18,40 @@ class LocalStoredPlaylistsControllerTest(
 
     def test_created_playlist_is_persisted(self):
         path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
-        self.assert_(not os.path.exists(path))
-        self.stored.create('test')
-        self.assert_(os.path.exists(path))
+        self.assertFalse(os.path.exists(path))
+
+        self.stored.create(u'test')
+        self.assertTrue(os.path.exists(path))
+
+    def test_create_slugifies_playlist_name(self):
+        path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test-foo-bar.m3u')
+        self.assertFalse(os.path.exists(path))
+
+        playlist = self.stored.create(u'test FOO baR')
+        self.assertEqual(u'test-foo-bar', playlist.name)
+        self.assertTrue(os.path.exists(path))
+
+    def test_create_slugifies_names_which_tries_to_change_directory(self):
+        path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test-foo-bar.m3u')
+        self.assertFalse(os.path.exists(path))
+
+        playlist = self.stored.create(u'../../test FOO baR')
+        self.assertEqual(u'test-foo-bar', playlist.name)
+        self.assertTrue(os.path.exists(path))
 
     def test_saved_playlist_is_persisted(self):
         path1 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test1.m3u')
-        path2 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2.m3u')
+        path2 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2-foo-bar.m3u')
 
-        playlist = self.stored.create('test1')
+        playlist = self.stored.create(u'test1')
 
         self.assertTrue(os.path.exists(path1))
         self.assertFalse(os.path.exists(path2))
 
-        playlist = playlist.copy(name='test2')
+        playlist = playlist.copy(name=u'test2 FOO baR')
         playlist = self.stored.save(playlist)
 
+        self.assertEqual(u'test2-foo-bar', playlist.name)
         self.assertFalse(os.path.exists(path1))
         self.assertTrue(os.path.exists(path2))
 
@@ -41,7 +59,7 @@ class LocalStoredPlaylistsControllerTest(
         path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
         self.assertFalse(os.path.exists(path))
 
-        playlist = self.stored.create('test')
+        playlist = self.stored.create(u'test')
         self.assertTrue(os.path.exists(path))
 
         self.stored.delete(playlist.uri)
@@ -50,7 +68,7 @@ class LocalStoredPlaylistsControllerTest(
     def test_playlist_contents_is_written_to_disk(self):
         track = Track(uri=generate_song(1))
         track_path = track.uri[len('file://'):]
-        playlist = self.stored.create('test')
+        playlist = self.stored.create(u'test')
         playlist_path = playlist.uri[len('file://'):]
         playlist = playlist.copy(tracks=[track])
         playlist = self.stored.save(playlist)
@@ -64,7 +82,7 @@ class LocalStoredPlaylistsControllerTest(
         playlist_path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
 
         track = Track(uri=path_to_uri(path_to_data_dir('uri2')))
-        playlist = self.stored.create('test')
+        playlist = self.stored.create(u'test')
         playlist = playlist.copy(tracks=[track])
         playlist = self.stored.save(playlist)
 

From 82f5b376da73c06c4023572c27be60117e300eb5 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Thu, 1 Nov 2012 14:03:09 +0100
Subject: [PATCH 15/19] Validate the stored playlist file paths

---
 mopidy/backends/local/stored_playlists.py | 73 +++++++++++++++++------
 1 file changed, 54 insertions(+), 19 deletions(-)

diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py
index 3d488655..9e5b72c6 100644
--- a/mopidy/backends/local/stored_playlists.py
+++ b/mopidy/backends/local/stored_playlists.py
@@ -19,16 +19,14 @@ logger = logging.getLogger(u'mopidy.backends.local')
 class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
     def __init__(self, *args, **kwargs):
         super(LocalStoredPlaylistsProvider, self).__init__(*args, **kwargs)
-        self._folder = settings.LOCAL_PLAYLIST_PATH
+        self._path = settings.LOCAL_PLAYLIST_PATH
         self.refresh()
 
     def create(self, name):
         name = self._slugify(name)
-        file_path = os.path.join(self._folder, name + '.m3u')
-        uri = path.path_to_uri(file_path)
+        uri = path.path_to_uri(self._get_m3u_path(name))
         playlist = Playlist(uri=uri, name=name)
-        self.save(playlist)
-        return playlist
+        return self.save(playlist)
 
     def delete(self, uri):
         playlist = self.lookup(uri)
@@ -36,7 +34,7 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
             return
 
         self._playlists.remove(playlist)
-        self._delete_m3u(playlist)
+        self._delete_m3u(playlist.uri)
 
     def lookup(self, uri):
         for playlist in self._playlists:
@@ -44,21 +42,24 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
                 return playlist
 
     def refresh(self):
+        logger.info('Loading playlists from %s', self._path)
+
         playlists = []
 
-        logger.info('Loading playlists from %s', self._folder)
-
-        for m3u in glob.glob(os.path.join(self._folder, '*.m3u')):
+        for m3u in glob.glob(os.path.join(self._path, '*.m3u')):
             uri = path.path_to_uri(m3u)
             name = os.path.splitext(os.path.basename(m3u))[0]
+
             tracks = []
             for track_uri in parse_m3u(m3u, settings.LOCAL_MUSIC_PATH):
                 try:
+                    # TODO We must use core.library.lookup() to support tracks
+                    # from other backends
                     tracks.append(self.backend.library.lookup(track_uri))
                 except LookupError as ex:
                     logger.error('Playlist item could not be added: %s', ex)
-            playlist = Playlist(uri=uri, name=name, tracks=tracks)
 
+            playlist = Playlist(uri=uri, name=name, tracks=tracks)
             playlists.append(playlist)
 
         self.playlists = playlists
@@ -69,11 +70,8 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         old_playlist = self.lookup(playlist.uri)
 
         if old_playlist and playlist.name != old_playlist.name:
-            new_name = self._slugify(playlist.name)
-            src = os.path.join(self._folder, old_playlist.name + '.m3u')
-            dst = os.path.join(self._folder, new_name + '.m3u')
-            shutil.move(src, dst)
-            playlist = playlist.copy(uri=path.path_to_uri(dst), name=new_name)
+            playlist = playlist.copy(name=self._slugify(playlist.name))
+            playlist = self._rename_m3u(playlist)
 
         self._save_m3u(playlist)
 
@@ -85,21 +83,58 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
 
         return playlist
 
+    def _get_m3u_path(self, name):
+        name = self._slugify(name)
+        file_path = os.path.join(self._path, name + '.m3u')
+        self._validate_file_path(file_path)
+        return file_path
+
     def _save_m3u(self, playlist):
-        file_path = playlist.uri[len('file://'):]
+        file_path = path.uri_to_path(playlist.uri)
+        self._validate_file_path(file_path)
         with open(file_path, 'w') as file_handle:
             for track in playlist.tracks:
                 if track.uri.startswith('file://'):
-                    uri = track.uri[len('file://'):]
+                    uri = path.uri_to_path(track.uri)
                 else:
                     uri = track.uri
                 file_handle.write(uri + '\n')
 
-    def _delete_m3u(self, playlist):
-        file_path = playlist.uri[len('file://'):]
+    def _delete_m3u(self, uri):
+        file_path = path.uri_to_path(uri)
+        self._validate_file_path(file_path)
         if os.path.exists(file_path):
             os.remove(file_path)
 
+    def _rename_m3u(self, playlist):
+        src_file_path = path.uri_to_path(playlist.uri)
+        self._validate_file_path(src_file_path)
+
+        dst_file_path = self._get_m3u_path(playlist.name)
+        self._validate_file_path(dst_file_path)
+
+        shutil.move(src_file_path, dst_file_path)
+
+        return playlist.copy(uri=path.path_to_uri(dst_file_path))
+
+    def _validate_file_path(self, file_path):
+        assert not file_path.endswith(os.sep), (
+            'File path %s cannot end with a path separator' % file_path)
+
+        # Expand symlinks
+        real_base_path = os.path.realpath(self._path)
+        real_file_path = os.path.realpath(file_path)
+
+        # Use dir of file for prefix comparision, so we don't accept
+        # /tmp/foo.m3u as being inside /tmp/foo, simply because they have a
+        # common prefix, /tmp/foo, which matches the base path, /tmp/foo.
+        real_dir_path = os.path.dirname(real_file_path)
+
+        # Check if dir of file is the base path or a subdir
+        common_prefix = os.path.commonprefix([real_base_path, real_dir_path])
+        assert common_prefix == real_base_path, (
+            'File path %s must be in %s' % (real_file_path, real_base_path))
+
     def _slugify(self, value):
         """
         Converts to lowercase, removes non-word characters (alphanumerics and

From 3fe856c6ba3e6cd1444919c3eec0060d104e9079 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Thu, 1 Nov 2012 14:03:38 +0100
Subject: [PATCH 16/19] Mark regexp strings as raw to please pylint

---
 mopidy/backends/local/stored_playlists.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py
index 9e5b72c6..9615461c 100644
--- a/mopidy/backends/local/stored_playlists.py
+++ b/mopidy/backends/local/stored_playlists.py
@@ -145,5 +145,5 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         """
         value = unicodedata.normalize('NFKD', value)
         value = value.encode('ascii', 'ignore').decode('ascii')
-        value = re.sub('[^\w\s-]', '', value).strip().lower()
-        return re.sub('[-\s]+', '-', value)
+        value = re.sub(r'[^\w\s-]', '', value).strip().lower()
+        return re.sub(r'[-\s]+', '-', value)

From 0dd4aba1436d3d329d76127138831f70479e13b0 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Thu, 1 Nov 2012 23:03:29 +0100
Subject: [PATCH 17/19] Move slugify to mopidy.utils.formatting

---
 mopidy/backends/local/stored_playlists.py | 23 ++++-------------------
 mopidy/utils/formatting.py                | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py
index 9615461c..5f9a18a1 100644
--- a/mopidy/backends/local/stored_playlists.py
+++ b/mopidy/backends/local/stored_playlists.py
@@ -1,14 +1,12 @@
 import glob
 import logging
 import os
-import re
 import shutil
-import unicodedata
 
 from mopidy import settings
 from mopidy.backends import base
 from mopidy.models import Playlist
-from mopidy.utils import path
+from mopidy.utils import formatting, path
 
 from .translator import parse_m3u
 
@@ -23,7 +21,7 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         self.refresh()
 
     def create(self, name):
-        name = self._slugify(name)
+        name = formatting.slugify(name)
         uri = path.path_to_uri(self._get_m3u_path(name))
         playlist = Playlist(uri=uri, name=name)
         return self.save(playlist)
@@ -70,7 +68,7 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         old_playlist = self.lookup(playlist.uri)
 
         if old_playlist and playlist.name != old_playlist.name:
-            playlist = playlist.copy(name=self._slugify(playlist.name))
+            playlist = playlist.copy(name=formatting.slugify(playlist.name))
             playlist = self._rename_m3u(playlist)
 
         self._save_m3u(playlist)
@@ -84,7 +82,7 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         return playlist
 
     def _get_m3u_path(self, name):
-        name = self._slugify(name)
+        name = formatting.slugify(name)
         file_path = os.path.join(self._path, name + '.m3u')
         self._validate_file_path(file_path)
         return file_path
@@ -134,16 +132,3 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
         common_prefix = os.path.commonprefix([real_base_path, real_dir_path])
         assert common_prefix == real_base_path, (
             'File path %s must be in %s' % (real_file_path, real_base_path))
-
-    def _slugify(self, value):
-        """
-        Converts to lowercase, removes non-word characters (alphanumerics and
-        underscores) and converts spaces to hyphens. Also strips leading and
-        trailing whitespace.
-
-        This function is based on Django's slugify implementation.
-        """
-        value = unicodedata.normalize('NFKD', value)
-        value = value.encode('ascii', 'ignore').decode('ascii')
-        value = re.sub(r'[^\w\s-]', '', value).strip().lower()
-        return re.sub(r'[-\s]+', '-', value)
diff --git a/mopidy/utils/formatting.py b/mopidy/utils/formatting.py
index 46459959..9091bc2a 100644
--- a/mopidy/utils/formatting.py
+++ b/mopidy/utils/formatting.py
@@ -1,3 +1,7 @@
+import re
+import unicodedata
+
+
 def indent(string, places=4, linebreak='\n'):
     lines = string.split(linebreak)
     if len(lines) == 1:
@@ -6,3 +10,17 @@ def indent(string, places=4, linebreak='\n'):
     for line in lines:
         result += linebreak + ' ' * places + line
     return result
+
+
+def slugify(value):
+    """
+    Converts to lowercase, removes non-word characters (alphanumerics and
+    underscores) and converts spaces to hyphens. Also strips leading and
+    trailing whitespace.
+
+    This function is based on Django's slugify implementation.
+    """
+    value = unicodedata.normalize('NFKD', value)
+    value = value.encode('ascii', 'ignore').decode('ascii')
+    value = re.sub(r'[^\w\s-]', '', value).strip().lower()
+    return re.sub(r'[-\s]+', '-', value)

From b110e6a478bbd35e05aaa0e0e4e13c6fb6166969 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Thu, 1 Nov 2012 23:10:18 +0100
Subject: [PATCH 18/19] Move file path is in base dir checker to
 mopidy.utils.path

---
 mopidy/backends/local/stored_playlists.py | 28 ++++-------------------
 mopidy/utils/path.py                      | 19 +++++++++++++++
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py
index 5f9a18a1..04406c32 100644
--- a/mopidy/backends/local/stored_playlists.py
+++ b/mopidy/backends/local/stored_playlists.py
@@ -84,12 +84,12 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
     def _get_m3u_path(self, name):
         name = formatting.slugify(name)
         file_path = os.path.join(self._path, name + '.m3u')
-        self._validate_file_path(file_path)
+        path.check_file_path_is_inside_base_dir(file_path, self._path)
         return file_path
 
     def _save_m3u(self, playlist):
         file_path = path.uri_to_path(playlist.uri)
-        self._validate_file_path(file_path)
+        path.check_file_path_is_inside_base_dir(file_path, self._path)
         with open(file_path, 'w') as file_handle:
             for track in playlist.tracks:
                 if track.uri.startswith('file://'):
@@ -100,35 +100,17 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
 
     def _delete_m3u(self, uri):
         file_path = path.uri_to_path(uri)
-        self._validate_file_path(file_path)
+        path.check_file_path_is_inside_base_dir(file_path, self._path)
         if os.path.exists(file_path):
             os.remove(file_path)
 
     def _rename_m3u(self, playlist):
         src_file_path = path.uri_to_path(playlist.uri)
-        self._validate_file_path(src_file_path)
+        path.check_file_path_is_inside_base_dir(src_file_path, self._path)
 
         dst_file_path = self._get_m3u_path(playlist.name)
-        self._validate_file_path(dst_file_path)
+        path.check_file_path_is_inside_base_dir(dst_file_path, self._path)
 
         shutil.move(src_file_path, dst_file_path)
 
         return playlist.copy(uri=path.path_to_uri(dst_file_path))
-
-    def _validate_file_path(self, file_path):
-        assert not file_path.endswith(os.sep), (
-            'File path %s cannot end with a path separator' % file_path)
-
-        # Expand symlinks
-        real_base_path = os.path.realpath(self._path)
-        real_file_path = os.path.realpath(file_path)
-
-        # Use dir of file for prefix comparision, so we don't accept
-        # /tmp/foo.m3u as being inside /tmp/foo, simply because they have a
-        # common prefix, /tmp/foo, which matches the base path, /tmp/foo.
-        real_dir_path = os.path.dirname(real_file_path)
-
-        # Check if dir of file is the base path or a subdir
-        common_prefix = os.path.commonprefix([real_base_path, real_dir_path])
-        assert common_prefix == real_base_path, (
-            'File path %s must be in %s' % (real_file_path, real_base_path))
diff --git a/mopidy/utils/path.py b/mopidy/utils/path.py
index eef0c2db..1092534f 100644
--- a/mopidy/utils/path.py
+++ b/mopidy/utils/path.py
@@ -102,6 +102,25 @@ def find_files(path):
                 yield filename
 
 
+def check_file_path_is_inside_base_dir(file_path, base_path):
+    assert not file_path.endswith(os.sep), (
+        'File path %s cannot end with a path separator' % file_path)
+
+    # Expand symlinks
+    real_base_path = os.path.realpath(base_path)
+    real_file_path = os.path.realpath(file_path)
+
+    # Use dir of file for prefix comparision, so we don't accept
+    # /tmp/foo.m3u as being inside /tmp/foo, simply because they have a
+    # common prefix, /tmp/foo, which matches the base path, /tmp/foo.
+    real_dir_path = os.path.dirname(real_file_path)
+
+    # Check if dir of file is the base path or a subdir
+    common_prefix = os.path.commonprefix([real_base_path, real_dir_path])
+    assert common_prefix == real_base_path, (
+        'File path %s must be in %s' % (real_file_path, real_base_path))
+
+
 # FIXME replace with mock usage in tests.
 class Mtime(object):
     def __init__(self):

From f08fba954e9e266e5f98067cdde9d6e94c14c771 Mon Sep 17 00:00:00 2001
From: Stein Magnus Jodal <stein.magnus@jodal.no>
Date: Sat, 3 Nov 2012 22:03:26 +0100
Subject: [PATCH 19/19] Update to work with current develop

---
 tests/frontends/mpd/protocol/stored_playlists_test.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/frontends/mpd/protocol/stored_playlists_test.py b/tests/frontends/mpd/protocol/stored_playlists_test.py
index 8cfe237c..c8db3f8f 100644
--- a/tests/frontends/mpd/protocol/stored_playlists_test.py
+++ b/tests/frontends/mpd/protocol/stored_playlists_test.py
@@ -15,7 +15,7 @@ class StoredPlaylistsHandlerTest(protocol.BaseTestCase):
         self.assertInResponse(u'OK')
 
     def test_listplaylist_without_quotes(self):
-        self.core.stored_playlists.playlists = [
+        self.backend.stored_playlists.playlists = [
             Playlist(name='name', tracks=[Track(uri='file:///dev/urandom')])]
 
         self.sendRequest(u'listplaylist name')
@@ -37,7 +37,7 @@ class StoredPlaylistsHandlerTest(protocol.BaseTestCase):
         self.assertInResponse(u'OK')
 
     def test_listplaylistinfo_without_quotes(self):
-        self.core.stored_playlists.playlists = [
+        self.backend.stored_playlists.playlists = [
             Playlist(name='name', tracks=[Track(uri='file:///dev/urandom')])]
 
         self.sendRequest(u'listplaylistinfo name')