From 80df6d1feabe70a876ea1795a83c11b52eee1f41 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 2 Aug 2010 14:24:12 +0200 Subject: [PATCH 1/2] CurrentPlaylistCtl.add() returns the cp_track that was added to the current playlist --- docs/changes.rst | 2 ++ mopidy/backends/__init__.py | 8 ++++++-- tests/backends/base.py | 13 ++++++------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 9c426795..2cb979c5 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -50,6 +50,8 @@ We got an updated :doc:`release roadmap `! lists of :class:`mopidy.models.Track` instead of :class:`mopidy.models.Playlist`, as none of the other fields on the ``Playlist`` model was in use. + - :meth:`mopidy.backends.BaseCurrentPlaylistController.add()` now returns the + ``cp_track`` added to the current playlist. - :meth:`mopidy.backends.BaseCurrentPlaylistController.remove()` now takes criterias, just like :meth:`mopidy.backends.BaseCurrentPlaylistController.get()`. diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index e659fa80..1af6bb42 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -122,14 +122,18 @@ class BaseCurrentPlaylistController(object): :type track: :class:`mopidy.models.Track` :param at_position: position in current playlist to add track :type at_position: int or :class:`None` + :rtype: two-tuple of (CPID integer, :class:`mopidy.models.Track`) that + was added to the current playlist playlist """ assert at_position <= len(self._cp_tracks), \ u'at_position can not be greater than playlist length' + cp_track = (self.version, track) if at_position is not None: - self._cp_tracks.insert(at_position, (self.version, track)) + self._cp_tracks.insert(at_position, cp_track) else: - self._cp_tracks.append((self.version, track)) + self._cp_tracks.append(cp_track) self.version += 1 + return cp_track def clear(self): """Clear the current playlist.""" diff --git a/tests/backends/base.py b/tests/backends/base.py index a7d17a94..d00770e2 100644 --- a/tests/backends/base.py +++ b/tests/backends/base.py @@ -43,24 +43,23 @@ class BaseCurrentPlaylistControllerTest(object): def test_add(self): for track in self.tracks: - self.controller.add(track) + cp_track = self.controller.add(track) self.assertEqual(track, self.controller.tracks[-1]) + self.assertEqual(cp_track, self.controller.cp_tracks[-1]) + self.assertEqual(track, cp_track[1]) def test_add_at_position(self): for track in self.tracks[:-1]: - self.controller.add(track, 0) + cp_track = self.controller.add(track, 0) self.assertEqual(track, self.controller.tracks[0]) + self.assertEqual(cp_track, self.controller.cp_tracks[0]) + self.assertEqual(track, cp_track[1]) @populate_playlist def test_add_at_position_outside_of_playlist(self): test = lambda: self.controller.add(self.tracks[0], len(self.tracks)+2) self.assertRaises(AssertionError, test) - @populate_playlist - def test_add_sets_id_property(self): - for track in self.controller.tracks: - self.assertNotEqual(None, track.id) - @populate_playlist def test_get_by_cpid(self): cp_track = self.controller.cp_tracks[1] From 44309345db3f72721bd41bc23fe72bd325146e6b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 2 Aug 2010 14:27:57 +0200 Subject: [PATCH 2/2] Update 'addid' to return CPID instead of GID --- mopidy/mpd/frontend.py | 4 ++-- tests/mpd/current_playlist_test.py | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/mopidy/mpd/frontend.py b/mopidy/mpd/frontend.py index 7aa5d6e0..b8c4d098 100644 --- a/mopidy/mpd/frontend.py +++ b/mopidy/mpd/frontend.py @@ -273,8 +273,8 @@ class MpdFrontend(object): raise MpdNoExistError(u'No such song', command=u'addid') if songpos and songpos > len(self.backend.current_playlist.tracks): raise MpdArgError(u'Bad song index', command=u'addid') - self.backend.current_playlist.add(track, at_position=songpos) - return ('Id', track.id) + cp_track = self.backend.current_playlist.add(track, at_position=songpos) + return ('Id', cp_track[0]) @handle_pattern(r'^delete "(?P\d+):(?P\d+)*"$') def _current_playlist_delete_range(self, start, end=None): diff --git a/tests/mpd/current_playlist_test.py b/tests/mpd/current_playlist_test.py index 09e61b63..5a7b6ca4 100644 --- a/tests/mpd/current_playlist_test.py +++ b/tests/mpd/current_playlist_test.py @@ -29,7 +29,7 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): u'ACK [50@0] {add} directory or file not found') def test_addid_without_songpos(self): - needle = Track(uri='dummy://foo', id=137) + needle = Track(uri='dummy://foo') self.b.library._library = [Track(), Track(), needle, Track()] self.b.current_playlist.load( [Track(), Track(), Track(), Track(), Track()]) @@ -37,11 +37,12 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): result = self.h.handle_request(u'addid "dummy://foo"') self.assertEqual(len(self.b.current_playlist.tracks), 6) self.assertEqual(self.b.current_playlist.tracks[5], needle) - self.assert_(u'Id: 137' in result) + self.assert_(u'Id: %d' % self.b.current_playlist.cp_tracks[5][0] + in result) self.assert_(u'OK' in result) def test_addid_with_songpos(self): - needle = Track(uri='dummy://foo', id=137) + needle = Track(uri='dummy://foo') self.b.library._library = [Track(), Track(), needle, Track()] self.b.current_playlist.load( [Track(), Track(), Track(), Track(), Track()]) @@ -49,7 +50,8 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): result = self.h.handle_request(u'addid "dummy://foo" "3"') self.assertEqual(len(self.b.current_playlist.tracks), 6) self.assertEqual(self.b.current_playlist.tracks[3], needle) - self.assert_(u'Id: 137' in result) + self.assert_(u'Id: %d' % self.b.current_playlist.cp_tracks[3][0] + in result) self.assert_(u'OK' in result) def test_addid_with_songpos_out_of_bounds_should_ack(self):