Merge branch 'feature/mpd-playlist-mgmt' into develop

AUTHORS

@@ -72,3 +72,6 @@
 - Cadel Watson <cadel@cadelwatson.com>
 - Loïck Bonniot <git@lesterpig.com>
 - Gustaf Hallberg <ghallberg@gmail.com>
+- kozec <kozec@kozec.com>
+- Jelle van der Waa <jelle@vdwaa.nl>
+- Alex Malone <jalexmalone@gmail.com>

docs/changelog.rst

@@ -25,6 +25,18 @@ Local backend
 MPD frontend
 ------------
 
+- Implemented commands for modifying stored playlists:
+
+  - ``playlistadd``
+  - ``playlistclear``
+  - ``playlistdelete``
+  - ``playlistmove``
+  - ``rename``
+  - ``rm``
+  - ``save``
+
+  (Fixes: :issue:`1014`, PR: :issue:`1187`, :issue:`1308`, :issue:`1322`)
+
 - Start ``songid`` counting at 1 instead of 0 to match the original MPD server.
 
 Zeroconf

docs/ext/mpd.rst

@@ -45,7 +45,6 @@ Items on this list will probably not be supported in the near future.
 The following items are currently not supported, but should be added in the
 near future:
 
-- Modifying stored playlists is not supported
 - ``tagtypes`` is not supported
 - Live update of the music database is not supported
 

mopidy/mpd/__init__.py

@@ -25,6 +25,7 @@ class Extension(ext.Extension):
         schema['connection_timeout'] = config.Integer(minimum=1)
         schema['zeroconf'] = config.String(optional=True)
         schema['command_blacklist'] = config.List(optional=True)
+        schema['default_playlist_scheme'] = config.String()
         return schema
 
     def validate_environment(self):

mopidy/mpd/actor.py

@@ -67,6 +67,12 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener):
     def tracklist_changed(self):
         self.send_idle('playlist')
 
+    def playlist_changed(self, playlist):
+        self.send_idle('stored_playlist')
+
+    def playlist_deleted(self, playlist):
+        self.send_idle('stored_playlist')
+
     def options_changed(self):
         self.send_idle('options')
 

mopidy/mpd/exceptions.py

@@ -92,6 +92,27 @@ class MpdNotImplemented(MpdAckError):
         self.message = 'Not implemented'
 
 
+class MpdInvalidTrackForPlaylist(MpdAckError):
+    # NOTE: This is a custom error for Mopidy that does not exist in MPD.
+    error_code = 0
+
+    def __init__(self, playlist_scheme, track_scheme, *args, **kwargs):
+        super(MpdInvalidTrackForPlaylist, self).__init__(*args, **kwargs)
+        self.message = (
+            'Playlist with scheme "%s" can\'t store track scheme "%s"' %
+            (playlist_scheme, track_scheme))
+
+
+class MpdFailedToSavePlaylist(MpdAckError):
+    # NOTE: This is a custom error for Mopidy that does not exist in MPD.
+    error_code = 0
+
+    def __init__(self, backend_scheme, *args, **kwargs):
+        super(MpdFailedToSavePlaylist, self).__init__(*args, **kwargs)
+        self.message = 'Backend with scheme "%s" failed to save playlist' % (
+            backend_scheme)
+
+
 class MpdDisabled(MpdAckError):
     # NOTE: This is a custom error for Mopidy that does not exist in MPD.
     error_code = 0

mopidy/mpd/ext.conf

@@ -7,3 +7,4 @@ max_connections = 20
 connection_timeout = 60
 zeroconf = Mopidy MPD server on $hostname
 command_blacklist = listall,listallinfo
+default_playlist_scheme = m3u

mopidy/mpd/protocol/stored_playlists.py

@@ -1,10 +1,14 @@
 from __future__ import absolute_import, division, unicode_literals
 
 import datetime
+import logging
+import urlparse
 import warnings
 
 from mopidy.mpd import exceptions, protocol, translator
 
+logger = logging.getLogger(__name__)
+
 
 @protocol.commands.add('listplaylist')
 def listplaylist(context, name):
@@ -135,7 +139,7 @@ def load(context, name, playlist_slice=slice(0, None)):
 
 
 @protocol.commands.add('playlistadd')
-def playlistadd(context, name, uri):
+def playlistadd(context, name, track_uri):
     """
     *musicpd.org, stored playlists section:*
 
@@ -145,7 +149,63 @@ def playlistadd(context, name, uri):
 
         ``NAME.m3u`` will be created if it does not exist.
     """
-    raise exceptions.MpdNotImplemented  # TODO
+    uri = context.lookup_playlist_uri_from_name(name)
+    old_playlist = uri is not None and context.core.playlists.lookup(uri).get()
+    if not old_playlist:
+        # Create new playlist with this single track
+        lookup_res = context.core.library.lookup(uris=[track_uri]).get()
+        tracks = [
+            track
+            for uri_tracks in lookup_res.values()
+            for track in uri_tracks]
+        _create_playlist(context, name, tracks)
+    else:
+        # Add track to existing playlist
+        lookup_res = context.core.library.lookup(uris=[track_uri]).get()
+        new_tracks = [
+            track
+            for uri_tracks in lookup_res.values()
+            for track in uri_tracks]
+        new_playlist = old_playlist.replace(
+            tracks=list(old_playlist.tracks) + new_tracks)
+        saved_playlist = context.core.playlists.save(new_playlist).get()
+        if saved_playlist is None:
+            playlist_scheme = urlparse.urlparse(old_playlist.uri).scheme
+            uri_scheme = urlparse.urlparse(track_uri).scheme
+            raise exceptions.MpdInvalidTrackForPlaylist(
+                playlist_scheme, uri_scheme)
+
+
+def _create_playlist(context, name, tracks):
+    """
+    Creates new playlist using backend appropriate for the given tracks
+    """
+    uri_schemes = set([urlparse.urlparse(t.uri).scheme for t in tracks])
+    for scheme in uri_schemes:
+        new_playlist = context.core.playlists.create(name, scheme).get()
+        if new_playlist is None:
+            logger.debug(
+                "Backend for scheme %s can't create playlists", scheme)
+            continue  # Backend can't create playlists at all
+        new_playlist = new_playlist.replace(tracks=tracks)
+        saved_playlist = context.core.playlists.save(new_playlist).get()
+        if saved_playlist is not None:
+            return  # Created and saved
+        else:
+            continue  # Failed to save using this backend
+    # Can't use backend appropriate for passed URI schemes, use default one
+    default_scheme = context.dispatcher.config[
+        'mpd']['default_playlist_scheme']
+    new_playlist = context.core.playlists.create(name, default_scheme).get()
+    if new_playlist is None:
+        # If even MPD's default backend can't save playlist, everything is lost
+        logger.warning("MPD's default backend can't create playlists")
+        raise exceptions.MpdFailedToSavePlaylist(default_scheme)
+    new_playlist = new_playlist.replace(tracks=tracks)
+    saved_playlist = context.core.playlists.save(new_playlist).get()
+    if saved_playlist is None:
+        uri_scheme = urlparse.urlparse(new_playlist.uri).scheme
+        raise exceptions.MpdFailedToSavePlaylist(uri_scheme)
 
 
 @protocol.commands.add('playlistclear')
@@ -156,8 +216,18 @@ def playlistclear(context, name):
         ``playlistclear {NAME}``
 
         Clears the playlist ``NAME.m3u``.
+
+    The playlist will be created if it does not exist.
     """
-    raise exceptions.MpdNotImplemented  # TODO
+    uri = context.lookup_playlist_uri_from_name(name)
+    playlist = uri is not None and context.core.playlists.lookup(uri).get()
+    if not playlist:
+        playlist = context.core.playlists.create(name).get()
+
+    # Just replace tracks with empty list and save
+    playlist = playlist.replace(tracks=[])
+    if context.core.playlists.save(playlist).get() is None:
+        raise exceptions.MpdFailedToSavePlaylist(urlparse.urlparse(uri).scheme)
 
 
 @protocol.commands.add('playlistdelete', songpos=protocol.UINT)
@@ -169,7 +239,20 @@ def playlistdelete(context, name, songpos):
 
         Deletes ``SONGPOS`` from the playlist ``NAME.m3u``.
     """
-    raise exceptions.MpdNotImplemented  # TODO
+    uri = context.lookup_playlist_uri_from_name(name)
+    playlist = uri is not None and context.core.playlists.lookup(uri).get()
+    if not playlist:
+        raise exceptions.MpdNoExistError('No such playlist')
+
+    # Convert tracks to list and remove requested
+    tracks = list(playlist.tracks)
+    tracks.pop(songpos)
+
+    # Replace tracks and save playlist
+    playlist = playlist.replace(tracks=tracks)
+    saved_playlist = context.core.playlists.save(playlist).get()
+    if saved_playlist is None:
+        raise exceptions.MpdFailedToSavePlaylist(urlparse.urlparse(uri).scheme)
 
 
 @protocol.commands.add(
@@ -189,7 +272,23 @@ def playlistmove(context, name, from_pos, to_pos):
       documentation, but just the ``SONGPOS`` to move *from*, i.e.
       ``playlistmove {NAME} {FROM_SONGPOS} {TO_SONGPOS}``.
     """
-    raise exceptions.MpdNotImplemented  # TODO
+    uri = context.lookup_playlist_uri_from_name(name)
+    playlist = uri is not None and context.core.playlists.lookup(uri).get()
+    if not playlist:
+        raise exceptions.MpdNoExistError('No such playlist')
+    if from_pos == to_pos:
+        return  # Nothing to do
+
+    # Convert tracks to list and perform move
+    tracks = list(playlist.tracks)
+    track = tracks.pop(from_pos)
+    tracks.insert(to_pos, track)
+
+    # Replace tracks and save playlist
+    playlist = playlist.replace(tracks=tracks)
+    saved_playlist = context.core.playlists.save(playlist).get()
+    if saved_playlist is None:
+        raise exceptions.MpdFailedToSavePlaylist(urlparse.urlparse(uri).scheme)
 
 
 @protocol.commands.add('rename')
@@ -201,7 +300,19 @@ def rename(context, old_name, new_name):
 
         Renames the playlist ``NAME.m3u`` to ``NEW_NAME.m3u``.
     """
-    raise exceptions.MpdNotImplemented  # TODO
+    uri = context.lookup_playlist_uri_from_name(old_name)
+    uri_scheme = urlparse.urlparse(uri).scheme
+    old_playlist = uri is not None and context.core.playlists.lookup(uri).get()
+    if not old_playlist:
+        raise exceptions.MpdNoExistError('No such playlist')
+
+    # Create copy of the playlist and remove original
+    new_playlist = context.core.playlists.create(new_name, uri_scheme).get()
+    new_playlist = new_playlist.replace(tracks=old_playlist.tracks)
+    saved_playlist = context.core.playlists.save(new_playlist).get()
+    if saved_playlist is None:
+        raise exceptions.MpdFailedToSavePlaylist(uri_scheme)
+    context.core.playlists.delete(old_playlist.uri).get()
 
 
 @protocol.commands.add('rm')
@@ -213,7 +324,8 @@ def rm(context, name):
 
         Removes the playlist ``NAME.m3u`` from the playlist directory.
     """
-    raise exceptions.MpdNotImplemented  # TODO
+    uri = context.lookup_playlist_uri_from_name(name)
+    context.core.playlists.delete(uri).get()
 
 
 @protocol.commands.add('save')
@@ -226,4 +338,16 @@ def save(context, name):
         Saves the current playlist to ``NAME.m3u`` in the playlist
         directory.
     """
-    raise exceptions.MpdNotImplemented  # TODO
+    tracks = context.core.tracklist.get_tracks().get()
+    uri = context.lookup_playlist_uri_from_name(name)
+    playlist = uri is not None and context.core.playlists.lookup(uri).get()
+    if not playlist:
+        # Create new playlist
+        _create_playlist(context, name, tracks)
+    else:
+        # Overwrite existing playlist
+        new_playlist = playlist.replace(tracks=tracks)
+        saved_playlist = context.core.playlists.save(new_playlist).get()
+        if saved_playlist is None:
+            raise exceptions.MpdFailedToSavePlaylist(
+                urlparse.urlparse(uri).scheme)

tests/mpd/protocol/__init__.py

@@ -36,6 +36,7 @@ class BaseTestCase(unittest.TestCase):
             },
             'mpd': {
                 'password': None,
+                'default_playlist_scheme': 'dummy',
             }
         }
 

tests/mpd/protocol/test_stored_playlists.py

@@ -16,6 +16,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
                 name='name', uri='dummy:name', tracks=[Track(uri='dummy:a')])])
 
         self.send_request('listplaylist "name"')
+
         self.assertInResponse('file: dummy:a')
         self.assertInResponse('OK')
 
@@ -25,11 +26,13 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
                 name='name', uri='dummy:name', tracks=[Track(uri='dummy:a')])])
 
         self.send_request('listplaylist name')
+
         self.assertInResponse('file: dummy:a')
         self.assertInResponse('OK')
 
     def test_listplaylist_fails_if_no_playlist_is_found(self):
         self.send_request('listplaylist "name"')
+
         self.assertEqualResponse('ACK [50@0] {listplaylist} No such playlist')
 
     def test_listplaylist_duplicate(self):
@@ -38,6 +41,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
         self.backend.playlists.set_dummy_playlists([playlist1, playlist2])
 
         self.send_request('listplaylist "a [2]"')
+
         self.assertInResponse('file: c')
         self.assertInResponse('OK')
 
@@ -47,6 +51,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
                 name='name', uri='dummy:name', tracks=[Track(uri='dummy:a')])])
 
         self.send_request('listplaylistinfo "name"')
+
         self.assertInResponse('file: dummy:a')
         self.assertNotInResponse('Track: 0')
         self.assertNotInResponse('Pos: 0')
@@ -58,6 +63,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
                 name='name', uri='dummy:name', tracks=[Track(uri='dummy:a')])])
 
         self.send_request('listplaylistinfo name')
+
         self.assertInResponse('file: dummy:a')
         self.assertNotInResponse('Track: 0')
         self.assertNotInResponse('Pos: 0')
@@ -65,6 +71,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
 
     def test_listplaylistinfo_fails_if_no_playlist_is_found(self):
         self.send_request('listplaylistinfo "name"')
+
         self.assertEqualResponse(
             'ACK [50@0] {listplaylistinfo} No such playlist')
 
@@ -74,6 +81,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
         self.backend.playlists.set_dummy_playlists([playlist1, playlist2])
 
         self.send_request('listplaylistinfo "a [2]"')
+
         self.assertInResponse('file: c')
         self.assertNotInResponse('Track: 0')
         self.assertNotInResponse('Pos: 0')
@@ -86,6 +94,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
             Playlist(name='a', uri='dummy:a')])
 
         self.send_request('listplaylists')
+
         self.assertInResponse('playlist: a')
         # Date without milliseconds and with time zone information
         self.assertInResponse('Last-Modified: 2015-08-05T22:51:06Z')
@@ -97,6 +106,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
         self.backend.playlists.set_dummy_playlists([playlist1, playlist2])
 
         self.send_request('listplaylists')
+
         self.assertInResponse('playlist: a')
         self.assertInResponse('playlist: a [2]')
         self.assertInResponse('OK')
@@ -107,13 +117,16 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
             Playlist(name='', uri='dummy:', last_modified=last_modified)])
 
         self.send_request('listplaylists')
+
         self.assertNotInResponse('playlist: ')
         self.assertInResponse('OK')
 
     def test_listplaylists_replaces_newline_with_space(self):
         self.backend.playlists.set_dummy_playlists([
             Playlist(name='a\n', uri='dummy:')])
+
         self.send_request('listplaylists')
+
         self.assertInResponse('playlist: a ')
         self.assertNotInResponse('playlist: a\n')
         self.assertInResponse('OK')
@@ -121,7 +134,9 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
     def test_listplaylists_replaces_carriage_return_with_space(self):
         self.backend.playlists.set_dummy_playlists([
             Playlist(name='a\r', uri='dummy:')])
+
         self.send_request('listplaylists')
+
         self.assertInResponse('playlist: a ')
         self.assertNotInResponse('playlist: a\r')
         self.assertInResponse('OK')
@@ -129,7 +144,9 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
     def test_listplaylists_replaces_forward_slash_with_pipe(self):
         self.backend.playlists.set_dummy_playlists([
             Playlist(name='a/b', uri='dummy:')])
+
         self.send_request('listplaylists')
+
         self.assertInResponse('playlist: a|b')
         self.assertNotInResponse('playlist: a/b')
         self.assertInResponse('OK')
@@ -211,33 +228,110 @@ class PlaylistsHandlerTest(protocol.BaseTestCase):
 
     def test_load_unknown_playlist_acks(self):
         self.send_request('load "unknown playlist"')
+
         self.assertEqual(0, len(self.core.tracklist.tracks.get()))
         self.assertEqualResponse('ACK [50@0] {load} No such playlist')
 
     def test_playlistadd(self):
+        tracks = [
+            Track(uri='dummy:a'),
+            Track(uri='dummy:b'),
+        ]
+        self.backend.library.dummy_library = tracks
+        self.backend.playlists.set_dummy_playlists([
+            Playlist(
+                name='name', uri='dummy:a1', tracks=[tracks[0]])])
+
+        self.send_request('playlistadd "name" "dummy:b"')
+
+        self.assertInResponse('OK')
+        self.assertEqual(
+            2, len(self.backend.playlists.get_items('dummy:a1').get()))
+
+    def test_playlistadd_creates_playlist(self):
+        tracks = [
+            Track(uri='dummy:a'),
+        ]
+        self.backend.library.dummy_library = tracks
+
         self.send_request('playlistadd "name" "dummy:a"')
-        self.assertEqualResponse('ACK [0@0] {playlistadd} Not implemented')
+
+        self.assertInResponse('OK')
+        self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get())
 
     def test_playlistclear(self):
+        self.backend.playlists.set_dummy_playlists([
+            Playlist(
+                name='name', uri='dummy:a1', tracks=[Track(uri='b')])])
+
         self.send_request('playlistclear "name"')
-        self.assertEqualResponse('ACK [0@0] {playlistclear} Not implemented')
+
+        self.assertInResponse('OK')
+        self.assertEqual(
+            0, len(self.backend.playlists.get_items('dummy:a1').get()))
+
+    def test_playlistclear_creates_playlist(self):
+        self.send_request('playlistclear "name"')
+
+        self.assertInResponse('OK')
+        self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get())
 
     def test_playlistdelete(self):
-        self.send_request('playlistdelete "name" "5"')
+        tracks = [
-        self.assertEqualResponse('ACK [0@0] {playlistdelete} Not implemented')
+            Track(uri='dummy:a'),
+            Track(uri='dummy:b'),
+            Track(uri='dummy:c'),
+        ]   # len() == 3
+        self.backend.playlists.set_dummy_playlists([
+            Playlist(
+                name='name', uri='dummy:a1', tracks=tracks)])
+
+        self.send_request('playlistdelete "name" "2"')
+
+        self.assertInResponse('OK')
+        self.assertEqual(
+            2, len(self.backend.playlists.get_items('dummy:a1').get()))
 
     def test_playlistmove(self):
-        self.send_request('playlistmove "name" "5" "10"')
+        tracks = [
-        self.assertEqualResponse('ACK [0@0] {playlistmove} Not implemented')
+            Track(uri='dummy:a'),
+            Track(uri='dummy:b'),
+            Track(uri='dummy:c')    # this one is getting moved to top
+        ]
+        self.backend.playlists.set_dummy_playlists([
+            Playlist(
+                name='name', uri='dummy:a1', tracks=tracks)])
+
+        self.send_request('playlistmove "name" "2" "0"')
+
+        self.assertInResponse('OK')
+        self.assertEqual(
+            "dummy:c",
+            self.backend.playlists.get_items('dummy:a1').get()[0].uri)
 
     def test_rename(self):
+        self.backend.playlists.set_dummy_playlists([
+            Playlist(
+                name='old_name', uri='dummy:a1', tracks=[Track(uri='b')])])
+
         self.send_request('rename "old_name" "new_name"')
-        self.assertEqualResponse('ACK [0@0] {rename} Not implemented')
+
+        self.assertInResponse('OK')
+        self.assertIsNotNone(
+            self.backend.playlists.lookup('dummy:new_name').get())
 
     def test_rm(self):
+        self.backend.playlists.set_dummy_playlists([
+            Playlist(
+                name='name', uri='dummy:a1', tracks=[Track(uri='b')])])
+
         self.send_request('rm "name"')
-        self.assertEqualResponse('ACK [0@0] {rm} Not implemented')
+
+        self.assertInResponse('OK')
+        self.assertIsNone(self.backend.playlists.lookup('dummy:a1').get())
 
     def test_save(self):
         self.send_request('save "name"')
-        self.assertEqualResponse('ACK [0@0] {save} Not implemented')
+
+        self.assertInResponse('OK')
+        self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get())