Make local.stored_playlists.save() capable of renaming playlists (#217)

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**
 

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')

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)
 

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)