Fix playlist not found for disambiguated MPD playlists

mopidy/frontends/mpd/dispatcher.py

@@ -236,7 +236,7 @@ class MpdContext(object):
     #: The subsytems that we want to be notified about in idle mode.
     subscriptions = None
 
-    playlist_uri_from_name = None
+    _playlist_uri_from_name = None
     playlist_name_from_uri = None
 
     def __init__(self, dispatcher, session=None, config=None, core=None):
@@ -246,14 +246,14 @@ def __init__(self, dispatcher, session=None, config=None, core=None):
         self.core = core
         self.events = set()
         self.subscriptions = set()
-        self.playlist_uri_from_name = {}
+        self._playlist_uri_from_name = {}
         self.playlist_name_from_uri = {}
         self.refresh_playlists_mapping()
 
     def create_unique_name(self, playlist_name):
         name = playlist_name
         i = 2
-        while name in self.playlist_uri_from_name:
+        while name in self._playlist_uri_from_name:
             name = '%s [%d]' % (playlist_name, i)
             i += 1
         return name
@@ -264,11 +264,22 @@ def refresh_playlists_mapping(self):
         MPD
         """
         if self.core is not None:
-            self.playlist_uri_from_name.clear()
+            self._playlist_uri_from_name.clear()
             self.playlist_name_from_uri.clear()
             for playlist in self.core.playlists.playlists.get():
                 if not playlist.name:
                     continue
                 name = self.create_unique_name(playlist.name)
-                self.playlist_uri_from_name[name] = playlist.uri
+                self._playlist_uri_from_name[name] = playlist.uri
                 self.playlist_name_from_uri[playlist.uri] = name
+
+    def lookup_playlist_from_name(self, name):
+        """
+        Helper function to retrieve a playlist from it's unique MPD name.
+        """
+        if not self._playlist_uri_from_name:
+            self.refresh_playlists_mapping()
+        if name not in self._playlist_uri_from_name:
+            return None;
+        uri = self._playlist_uri_from_name[name]
+        return self.core.playlists.lookup(uri).get()

mopidy/frontends/mpd/protocol/music_db.py

@@ -381,13 +381,8 @@ def searchaddpl(context, playlist_name, mpd_query):
         return
     results = context.core.library.search(**query).get()
 
-    if len(context.playlist_uri_from_name) == 0:
-        context.refresh_playlists_mapping()
-
-    if playlist_name in context.playlist_uri_from_name:
-        uri = context.playlist_uri_from_name[playlist_name]
-        playlist = context.core.playlists.lookup(uri).get()
-    else:
+    playlist = context.lookup_playlist_from_name(playlist_name)
+    if not playlist:
         playlist = context.core.playlists.create(playlist_name).get()
     tracks = list(playlist.tracks) + _get_tracks(results)
     playlist = playlist.copy(tracks=tracks)

mopidy/frontends/mpd/protocol/stored_playlists.py

@@ -23,10 +23,10 @@ def listplaylist(context, name):
         file: relative/path/to/file2.ogg
         file: relative/path/to/file3.mp3
     """
-    playlists = context.core.playlists.filter(name=name).get()
-    if not playlists:
+    playlist = context.lookup_playlist_from_name(name)
+    if not playlist:
         raise MpdNoExistError('No such playlist', command='listplaylist')
-    return ['file: %s' % t.uri for t in playlists[0].tracks]
+    return ['file: %s' % t.uri for t in playlist.tracks]
 
 
 @handle_request(r'^listplaylistinfo (?P<name>\w+)$')
@@ -44,10 +44,10 @@ def listplaylistinfo(context, name):
         Standard track listing, with fields: file, Time, Title, Date,
         Album, Artist, Track
     """
-    playlists = context.core.playlists.filter(name=name).get()
-    if not playlists:
+    playlist = context.lookup_playlist_from_name(name)
+    if not playlist:
         raise MpdNoExistError('No such playlist', command='listplaylistinfo')
-    return playlist_to_mpd_format(playlists[0])
+    return playlist_to_mpd_format(playlist)
 
 
 @handle_request(r'^listplaylists$')
@@ -117,14 +117,14 @@ def load(context, name, start=None, end=None):
     - MPD 0.17.1 does not fail if the specified range is outside the playlist,
       in either or both ends.
     """
-    playlists = context.core.playlists.filter(name=name).get()
-    if not playlists:
+    playlist = context.lookup_playlist_from_name(name)
+    if not playlist:
         raise MpdNoExistError('No such playlist', command='load')
     if start is not None:
         start = int(start)
     if end is not None:
         end = int(end)
-    context.core.tracklist.add(playlists[0].tracks[start:end])
+    context.core.tracklist.add(playlist.tracks[start:end])
 
 
 @handle_request(r'^playlistadd "(?P<name>[^"]+)" "(?P<uri>[^"]+)"$')

tests/frontends/mpd/protocol/stored_playlists_test.py

@@ -10,15 +10,17 @@
 class PlaylistsHandlerTest(protocol.BaseTestCase):
     def test_listplaylist(self):
         self.backend.playlists.playlists = [
-            Playlist(name='name', tracks=[Track(uri='dummy:a')])]
+            Playlist(name='name', uri='dummy:name',
+                tracks=[Track(uri='dummy:a')])]
 
         self.sendRequest('listplaylist "name"')
         self.assertInResponse('file: dummy:a')
         self.assertInResponse('OK')
 
     def test_listplaylist_without_quotes(self):
         self.backend.playlists.playlists = [
-            Playlist(name='name', tracks=[Track(uri='dummy:a')])]
+            Playlist(name='name', uri='dummy:name', 
+                tracks=[Track(uri='dummy:a')])]
 
         self.sendRequest('listplaylist name')
         self.assertInResponse('file: dummy:a')
@@ -28,9 +30,19 @@ def test_listplaylist_fails_if_no_playlist_is_found(self):
         self.sendRequest('listplaylist "name"')
         self.assertEqualResponse('ACK [50@0] {listplaylist} No such playlist')
 
+    def test_listplaylist_duplicate(self):
+        playlist1 = Playlist(name='a', uri='dummy:a1', tracks=[Track(uri='b')])
+        playlist2 = Playlist(name='a', uri='dummy:a2', tracks=[Track(uri='c')])
+        self.backend.playlists.playlists = [playlist1, playlist2]
+
+        self.sendRequest('listplaylist "a [2]"')
+        self.assertInResponse('file: c')
+        self.assertInResponse('OK')
+
     def test_listplaylistinfo(self):
         self.backend.playlists.playlists = [
-            Playlist(name='name', tracks=[Track(uri='dummy:a')])]
+            Playlist(name='name', uri='dummy:name', 
+                tracks=[Track(uri='dummy:a')])]
 
         self.sendRequest('listplaylistinfo "name"')
         self.assertInResponse('file: dummy:a')
@@ -40,7 +52,8 @@ def test_listplaylistinfo(self):
 
     def test_listplaylistinfo_without_quotes(self):
         self.backend.playlists.playlists = [
-            Playlist(name='name', tracks=[Track(uri='dummy:a')])]
+            Playlist(name='name', uri='dummy:name', 
+                tracks=[Track(uri='dummy:a')])]
 
         self.sendRequest('listplaylistinfo name')
         self.assertInResponse('file: dummy:a')
@@ -53,10 +66,21 @@ def test_listplaylistinfo_fails_if_no_playlist_is_found(self):
         self.assertEqualResponse(
             'ACK [50@0] {listplaylistinfo} No such playlist')
 
+    def test_listplaylistinfo_duplicate(self):
+        playlist1 = Playlist(name='a', uri='dummy:a1', tracks=[Track(uri='b')])
+        playlist2 = Playlist(name='a', uri='dummy:a2', tracks=[Track(uri='c')])
+        self.backend.playlists.playlists = [playlist1, playlist2]
+
+        self.sendRequest('listplaylistinfo "a [2]"')
+        self.assertInResponse('file: c')
+        self.assertInResponse('Track: 0')
+        self.assertNotInResponse('Pos: 0')
+        self.assertInResponse('OK')
+
     def test_listplaylists(self):
         last_modified = datetime.datetime(2001, 3, 17, 13, 41, 17, 12345)
         self.backend.playlists.playlists = [
-            Playlist(name='a', last_modified=last_modified)]
+            Playlist(name='a', uri='dummy:a', last_modified=last_modified)]
 
         self.sendRequest('listplaylists')
         self.assertInResponse('playlist: a')
@@ -77,7 +101,7 @@ def test_listplaylists_duplicate(self):
     def test_listplaylists_ignores_playlists_without_name(self):
         last_modified = datetime.datetime(2001, 3, 17, 13, 41, 17, 12345)
         self.backend.playlists.playlists = [
-            Playlist(name='', last_modified=last_modified)]
+            Playlist(name='', uri='dummy:', last_modified=last_modified)]
 
         self.sendRequest('listplaylists')
         self.assertNotInResponse('playlist: ')
@@ -87,7 +111,7 @@ def test_load_appends_to_tracklist(self):
         self.core.tracklist.add([Track(uri='a'), Track(uri='b')])
         self.assertEqual(len(self.core.tracklist.tracks.get()), 2)
         self.backend.playlists.playlists = [
-            Playlist(name='A-list', tracks=[
+            Playlist(name='A-list', uri='dummy:A-list', tracks=[
                 Track(uri='c'), Track(uri='d'), Track(uri='e')])]
 
         self.sendRequest('load "A-list"')
@@ -105,7 +129,7 @@ def test_load_with_range_loads_part_of_playlist(self):
         self.core.tracklist.add([Track(uri='a'), Track(uri='b')])
         self.assertEqual(len(self.core.tracklist.tracks.get()), 2)
         self.backend.playlists.playlists = [
-            Playlist(name='A-list', tracks=[
+            Playlist(name='A-list', uri='dummy:A-list', tracks=[
                 Track(uri='c'), Track(uri='d'), Track(uri='e')])]
 
         self.sendRequest('load "A-list" "1:2"')
@@ -121,7 +145,7 @@ def test_load_with_range_without_end_loads_rest_of_playlist(self):
         self.core.tracklist.add([Track(uri='a'), Track(uri='b')])
         self.assertEqual(len(self.core.tracklist.tracks.get()), 2)
         self.backend.playlists.playlists = [
-            Playlist(name='A-list', tracks=[
+            Playlist(name='A-list', uri='dummy:A-list', tracks=[
                 Track(uri='c'), Track(uri='d'), Track(uri='e')])]
 
         self.sendRequest('load "A-list" "1:"')