Merge branch 'v1.0.x' into develop

docs/changelog.rst

@@ -51,7 +51,7 @@ Internal changes
   :issue:`1115`)
 
 
-v1.0.1 (UNRELEASED)
+v1.0.1 (2015-04-23)
 ===================
 
 Bug fix release.
@@ -60,7 +60,7 @@ Bug fix release.
 
 - Audio: Software volume control has been reworked to greatly reduce the delay
   between changing the volume and the change taking effect. (Fixes:
-  :issue:`1097`)
+  :issue:`1097`, PR: :issue:`1101`)
 
 - Audio: As a side effect of the previous bug fix, software volume is no longer
   tied to the PulseAudio application volume when using ``pulsesink``. This
@@ -69,9 +69,15 @@ Bug fix release.
 
 - Audio: Update scanner to decode all media it finds. This should fix cases
   where the scanner hangs on non-audio files like video. The scanner will now
-  also let us know if we found any decodeable audio. (Fixes: :issue:`726`)
+  also let us know if we found any decodeable audio. (Fixes: :issue:`726`, PR:
+  issue:`1124`)
 
 - HTTP: Fix threading bug that would cause duplicate delivery of WS messages.
+  (PR: :issue:`1127`)
+
+- MPD: Fix case where a playlist that is present in both browse and as a listed
+  playlist breaks the MPD frontend protocol output. (Fixes :issue:`1120`, PR:
+  :issue:`1142`)
 
 
 v1.0.0 (2015-03-25)

mopidy/__init__.py

@@ -14,4 +14,4 @@ if not (2, 7) <= sys.version_info < (3,):
 warnings.filterwarnings('ignore', 'could not open display')
 
 
-__version__ = '1.0.0'
+__version__ = '1.0.1'

mopidy/mpd/uri_mapper.py

@@ -2,6 +2,9 @@ from __future__ import absolute_import, unicode_literals
 
 import re
 
+# TOOD: refactor this into a generic mapper that does not know about browse
+# or playlists and then use one instance for each case?
+
 
 class MpdUriMapper(object):
 
@@ -18,7 +21,8 @@ class MpdUriMapper(object):
     def __init__(self, core=None):
         self.core = core
         self._uri_from_name = {}
-        self._name_from_uri = {}
+        self._browse_name_from_uri = {}
+        self._playlist_name_from_uri = {}
 
     def _create_unique_name(self, name, uri):
         stripped_name = self._invalid_browse_chars.sub(' ', name)
@@ -31,33 +35,37 @@ class MpdUriMapper(object):
             i += 1
         return name
 
-    def insert(self, name, uri):
+    def insert(self, name, uri, playlist=False):
         """
         Create a unique and MPD compatible name that maps to the given URI.
         """
         name = self._create_unique_name(name, uri)
         self._uri_from_name[name] = uri
-        self._name_from_uri[uri] = name
+        if playlist:
+            self._playlist_name_from_uri[uri] = name
+        else:
+            self._browse_name_from_uri[uri] = name
         return name
 
     def uri_from_name(self, name):
         """
         Return the uri for the given MPD name.
         """
-        if name in self._uri_from_name:
-            return self._uri_from_name[name]
+        return self._uri_from_name.get(name)
 
     def refresh_playlists_mapping(self):
         """
         Maintain map between playlists and unique playlist names to be used by
         MPD.
         """
-        if self.core is not None:
-            for playlist_ref in self.core.playlists.as_list().get():
-                if not playlist_ref.name:
-                    continue
-                name = self._invalid_playlist_chars.sub('|', playlist_ref.name)
-                self.insert(name, playlist_ref.uri)
+        if self.core is None:
+            return
+
+        for playlist_ref in self.core.playlists.as_list().get():
+            if not playlist_ref.name:
+                continue
+            name = self._invalid_playlist_chars.sub('|', playlist_ref.name)
+            self.insert(name, playlist_ref.uri, playlist=True)
 
     def playlist_uri_from_name(self, name):
         """
@@ -71,6 +79,6 @@ class MpdUriMapper(object):
         """
         Helper function to retrieve the unique MPD playlist name from its URI.
         """
-        if uri not in self._name_from_uri:
+        if uri not in self._playlist_name_from_uri:
             self.refresh_playlists_mapping()
-        return self._name_from_uri[uri]
+        return self._playlist_name_from_uri[uri]

tests/mpd/protocol/test_regression.py

@@ -2,7 +2,7 @@ from __future__ import absolute_import, unicode_literals
 
 import random
 
-from mopidy.models import Track
+from mopidy.models import Playlist, Ref, Track
 
 from tests.mpd import protocol
 
@@ -197,3 +197,33 @@ class IssueGH137RegressionTest(protocol.BaseTestCase):
             u'Album "This Is Remixed Hits - Mashups & Rare 12" Mixes"')
 
         self.assertInResponse('ACK [2@0] {list} Invalid unquoted character')
+
+
+class IssueGH1120RegressionTest(protocol.BaseTestCase):
+    """
+    The issue: https://github.com/mopidy/mopidy/issues/1120
+
+    How to reproduce:
+
+    - A playlist must be in both browse results and playlists
+    - Call for instance ``lsinfo "/"`` to populate the cache with the
+      playlist name from the playlist backend.
+    - Call ``lsinfo "/dummy"`` to override the playlist name with the browse
+      name.
+    - Call ``lsinfo "/"`` and we now have an invalid name with ``/`` in it.
+
+    """
+
+    def test(self):
+        self.backend.library.dummy_browse_result = {
+            'dummy:/': [Ref.playlist(name='Top 100 tracks', uri='dummy:/1')],
+        }
+        self.backend.playlists.set_dummy_playlists([
+            Playlist(name='Top 100 tracks', uri='dummy:/1'),
+        ])
+
+        response1 = self.send_request('lsinfo "/"')
+        self.send_request('lsinfo "/dummy"')
+
+        response2 = self.send_request('lsinfo "/"')
+        self.assertEqual(response1, response2)

tests/test_version.py

@@ -56,5 +56,6 @@ class VersionTest(unittest.TestCase):
         self.assertVersionLess('0.19.2', '0.19.3')
         self.assertVersionLess('0.19.3', '0.19.4')
         self.assertVersionLess('0.19.4', '0.19.5')
-        self.assertVersionLess('0.19.5', __version__)
-        self.assertVersionLess(__version__, '1.0.1')
+        self.assertVersionLess('0.19.5', '1.0.0')
+        self.assertVersionLess('1.0.0', __version__)
+        self.assertVersionLess(__version__, '1.0.2')