Fix GH-16: 'addid ""' crashes with SpotifyError

docs/changes.rst

@@ -5,6 +5,18 @@ Changes
 This change log is used to track all major changes to Mopidy.
 
 
+0.1.1 (in development)
+======================
+
+No description yet.
+
+**Changes**
+
+- MPD frontend:
+
+  - ``add ""`` and ``addid ""`` now behaves as expected.
+
+
 0.1.0 (2010-08-23)
 ==================
 

mopidy/backends/libspotify/library.py

@@ -1,7 +1,7 @@
 import logging
 import multiprocessing
 
-from spotify import Link
+from spotify import Link, SpotifyError
 
 from mopidy.backends.base import BaseLibraryController
 from mopidy.backends.libspotify import ENCODING
@@ -14,10 +14,15 @@ class LibspotifyLibraryController(BaseLibraryController):
         return self.search(**query)
 
     def lookup(self, uri):
-        spotify_track = Link.from_string(uri).as_track()
-        # TODO Block until metadata_updated callback is called. Before that the
-        # track will be unloaded, unless it's already in the stored playlists.
-        return LibspotifyTranslator.to_mopidy_track(spotify_track)
+        try:
+            spotify_track = Link.from_string(uri).as_track()
+            # TODO Block until metadata_updated callback is called. Before that
+            # the track will be unloaded, unless it's already in the stored
+            # playlists.
+            return LibspotifyTranslator.to_mopidy_track(spotify_track)
+        except SpotifyError as e:
+            logger.warning(u'Failed to lookup: %s', uri, e)
+            return None
 
     def refresh(self, uri=None):
         pass # TODO

mopidy/frontends/mpd/protocol/current_playlist.py

@@ -11,14 +11,19 @@ def add(frontend, uri):
 
         Adds the file ``URI`` to the playlist (directories add recursively).
         ``URI`` can also be a single file.
+
+    *Clarifications:*
+
+    - ``add ""`` should add all tracks in the library to the current playlist.
     """
+    if not uri:
+        return
     for handler_prefix in frontend.backend.uri_handlers:
         if uri.startswith(handler_prefix):
             track = frontend.backend.library.lookup(uri)
             if track is not None:
                 frontend.backend.current_playlist.add(track)
                 return
-
     raise MpdNoExistError(
         u'directory or file not found', command=u'add')
 
@@ -36,7 +41,13 @@ def addid(frontend, uri, songpos=None):
             addid "foo.mp3"
             Id: 999
             OK
+
+    *Clarifications:*
+
+    - ``addid ""`` should return an error.
     """
+    if not uri:
+        raise MpdNoExistError(u'No such song', command=u'addid')
     if songpos is not None:
         songpos = int(songpos)
     track = frontend.backend.library.lookup(uri)
@@ -44,7 +55,8 @@ def addid(frontend, uri, songpos=None):
         raise MpdNoExistError(u'No such song', command=u'addid')
     if songpos and songpos > len(frontend.backend.current_playlist.tracks):
         raise MpdArgError(u'Bad song index', command=u'addid')
-    cp_track = frontend.backend.current_playlist.add(track, at_position=songpos)
+    cp_track = frontend.backend.current_playlist.add(track,
+        at_position=songpos)
     return ('Id', cp_track[0])
 
 @handle_pattern(r'^delete "(?P<start>\d+):(?P<end>\d+)*"$')

tests/frontends/mpd/current_playlist_test.py

@@ -33,6 +33,11 @@ class CurrentPlaylistHandlerTest(unittest.TestCase):
         self.assertEqual(result[0],
             u'ACK [50@0] {add} directory or file not found')
 
+    def test_add_with_empty_uri_should_add_all_known_tracks_and_ok(self):
+        result = self.h.handle_request(u'add ""')
+        # TODO check that we add all tracks (we currently don't)
+        self.assert_(u'OK' in result)
+
     def test_addid_without_songpos(self):
         needle = Track(uri='dummy://foo')
         self.b.library._library = [Track(), Track(), needle, Track()]
@@ -46,6 +51,11 @@ class CurrentPlaylistHandlerTest(unittest.TestCase):
             in result)
         self.assert_(u'OK' in result)
 
+    def test_addid_with_empty_uri_does_not_lookup_and_acks(self):
+        self.b.library.lookup = lambda uri: self.fail("Shouldn't run")
+        result = self.h.handle_request(u'addid ""')
+        self.assertEqual(result[0], u'ACK [50@0] {addid} No such song')
+
     def test_addid_with_songpos(self):
         needle = Track(uri='dummy://foo')
         self.b.library._library = [Track(), Track(), needle, Track()]