From 4ba30f80e49f5c134baff3cff2ec1c4a322e59f1 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 29 Jun 2010 23:53:52 +0200 Subject: [PATCH 01/13] CurrentPlaylistController.load() takes a list of tracks instead of a Playlist object --- docs/changes.rst | 7 + mopidy/backends/__init__.py | 58 ++++---- mopidy/mpd/frontend.py | 43 +++--- tests/backends/base.py | 115 +++++++-------- tests/mpd/frontend_test.py | 286 +++++++++++++++++++----------------- 5 files changed, 259 insertions(+), 250 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index bc28d4b8..267df472 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -25,6 +25,13 @@ We got an updated :doc:`release roadmap `! - ``command_list_end`` before ``command_list_start`` now returns unknown command error instead of crashing. +- Backend API: + + - :meth:`mopidy.backends.BaseCurrentPlaylistController.load()` now accepts + 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. + 0.1.0a2 (2010-06-02) ==================== diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index 9b08250b..3983d94a 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -88,20 +88,20 @@ class BaseCurrentPlaylistController(object): def __init__(self, backend): self.backend = backend - self._playlist = Playlist() + self._tracks = [] def destroy(self): """Cleanup after component.""" pass @property - def playlist(self): - """The currently loaded :class:`mopidy.models.Playlist`.""" - return copy(self._playlist) + def tracks(self): + """List of :class:`mopidy.model.Track` in the current playlist.""" + return copy(self._tracks) - @playlist.setter - def playlist(self, new_playlist): - self._playlist = new_playlist + @tracks.setter + def tracks(self, new_tracks): + self._tracks = copy(new_tracks) self.version += 1 def add(self, track, at_position=None): @@ -114,7 +114,7 @@ class BaseCurrentPlaylistController(object): :param at_position: position in current playlist to add track :type at_position: int or :class:`None` """ - tracks = self.playlist.tracks + tracks = self.tracks assert at_position <= len(tracks), 'at_position can not be greater' \ + ' than playlist length' @@ -123,13 +123,13 @@ class BaseCurrentPlaylistController(object): tracks.insert(at_position, track) else: tracks.append(track) - self.playlist = self.playlist.with_(tracks=tracks) + self.tracks = tracks def clear(self): """Clear the current playlist.""" self.backend.playback.stop() self.backend.playback.current_track = None - self.playlist = Playlist() + self.tracks = [] def get(self, **criteria): """ @@ -147,7 +147,7 @@ class BaseCurrentPlaylistController(object): :type criteria: dict :rtype: :class:`mopidy.models.Track` """ - matches = self._playlist.tracks + matches = self.tracks for (key, value) in criteria.iteritems(): matches = filter(lambda t: getattr(t, key) == value, matches) if len(matches) == 1: @@ -159,14 +159,14 @@ class BaseCurrentPlaylistController(object): else: raise LookupError(u'"%s" match multiple tracks' % criteria_string) - def load(self, playlist): + def load(self, tracks): """ - Replace the current playlist with the given playlist. + Replace the tracks in the current playlist with the given tracks. - :param playlist: playlist to load - :type playlist: :class:`mopidy.models.Playlist` + :param tracks: tracks to load + :type tracks: list of :class:`mopidy.models.Track` """ - self.playlist = playlist + self.tracks = tracks self.backend.playback.new_playlist_loaded_callback() def move(self, start, end, to_position): @@ -183,7 +183,7 @@ class BaseCurrentPlaylistController(object): if start == end: end += 1 - tracks = self.playlist.tracks + tracks = self.tracks assert start < end, 'start must be smaller than end' assert start >= 0, 'start must be at least zero' @@ -196,7 +196,7 @@ class BaseCurrentPlaylistController(object): for track in tracks[start:end]: new_tracks.insert(to_position, track) to_position += 1 - self.playlist = self.playlist.with_(tracks=new_tracks) + self.tracks = new_tracks def remove(self, track): """ @@ -205,13 +205,13 @@ class BaseCurrentPlaylistController(object): :param track: track to remove :type track: :class:`mopidy.models.Track` """ - tracks = self.playlist.tracks + tracks = self.tracks assert track in tracks, 'track must be in playlist' position = tracks.index(track) del tracks[position] - self.playlist = self.playlist.with_(tracks=tracks) + self.tracks = tracks def shuffle(self, start=None, end=None): """ @@ -223,7 +223,7 @@ class BaseCurrentPlaylistController(object): :param end: position after last track to shuffle :type end: int or :class:`None` """ - tracks = self.playlist.tracks + tracks = self.tracks if start is not None and end is not None: assert start < end, 'start must be smaller than end' @@ -239,12 +239,16 @@ class BaseCurrentPlaylistController(object): shuffled = tracks[start:end] after = tracks[end or len(tracks):] random.shuffle(shuffled) - self.playlist = self.playlist.with_(tracks=before+shuffled+after) + self.tracks = before + shuffled + after def destroy(self): """Cleanup after component.""" pass + def mpd_format(self, *args, **kwargs): + # XXX Lazy workaround to make tests pass while refactoring + return Playlist(tracks=self.tracks).mpd_format(*args, **kwargs) + class BaseLibraryController(object): """ @@ -364,7 +368,7 @@ class BasePlaybackController(object): if self.current_track is None: return None try: - return self.backend.current_playlist.playlist.tracks.index( + return self.backend.current_playlist.tracks.index( self.current_track) except ValueError: return None @@ -379,7 +383,7 @@ class BasePlaybackController(object): enabled this should be a random track, all tracks should be played once before the list repeats. """ - tracks = self.backend.current_playlist.playlist.tracks + tracks = self.backend.current_playlist.tracks if not tracks: return None @@ -420,7 +424,7 @@ class BasePlaybackController(object): if self.current_track is None or self.current_playlist_position == 0: return None - return self.backend.current_playlist.playlist.tracks[ + return self.backend.current_playlist.tracks[ self.current_playlist_position - 1] @property @@ -508,7 +512,7 @@ class BasePlaybackController(object): self._shuffled = [] if self.state == self.PLAYING: - if self.backend.current_playlist.playlist.length > 0: + if len(self.backend.current_playlist.tracks) > 0: self.play() else: self.stop() @@ -555,7 +559,7 @@ class BasePlaybackController(object): """ if track: - assert track in self.backend.current_playlist.playlist.tracks + assert track in self.backend.current_playlist.tracks elif not self.current_track: track = self.next_track diff --git a/mopidy/mpd/frontend.py b/mopidy/mpd/frontend.py index f80c4e2b..3259306e 100644 --- a/mopidy/mpd/frontend.py +++ b/mopidy/mpd/frontend.py @@ -271,7 +271,7 @@ class MpdFrontend(object): track = self.backend.library.lookup(uri) if track is None: raise MpdNoExistError(u'No such song', command=u'addid') - if songpos and songpos > self.backend.current_playlist.playlist.length: + 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) @@ -289,8 +289,8 @@ class MpdFrontend(object): if end is not None: end = int(end) else: - end = self.backend.current_playlist.playlist.length - tracks = self.backend.current_playlist.playlist.tracks[start:end] + end = len(self.backend.current_playlist.tracks) + tracks = self.backend.current_playlist.tracks[start:end] if not tracks: raise MpdArgError(u'Bad song index', command=u'delete') for track in tracks: @@ -301,7 +301,7 @@ class MpdFrontend(object): """See :meth:`_current_playlist_delete_range`""" try: songpos = int(songpos) - track = self.backend.current_playlist.playlist.tracks[songpos] + track = self.backend.current_playlist.tracks[songpos] self.backend.current_playlist.remove(track) except IndexError: raise MpdArgError(u'Bad song index', command=u'delete') @@ -344,7 +344,7 @@ class MpdFrontend(object): ``TO`` in the playlist. """ if end is None: - end = self.backend.current_playlist.playlist.length + end = len(self.backend.current_playlist.tracks) start = int(start) end = int(end) to = int(to) @@ -371,7 +371,7 @@ class MpdFrontend(object): songid = int(songid) to = int(to) track = self.backend.current_playlist.get(id=songid) - position = self.backend.current_playlist.playlist.tracks.index(track) + position = self.backend.current_playlist.tracks.index(track) self.backend.current_playlist.move(position, position + 1, to) @handle_pattern(r'^playlist$') @@ -429,7 +429,7 @@ class MpdFrontend(object): except LookupError: raise MpdNoExistError(u'No such song', command=u'playlistid') else: - return self.backend.current_playlist.playlist.mpd_format() + return self.backend.current_playlist.mpd_format() @handle_pattern(r'^playlistinfo$') @handle_pattern(r'^playlistinfo "(?P-?\d+)"$') @@ -459,15 +459,14 @@ class MpdFrontend(object): end = songpos + 1 if start == -1: end = None - return self.backend.current_playlist.playlist.mpd_format( - start, end) + return self.backend.current_playlist.mpd_format(start, end) else: if start is None: start = 0 start = int(start) if end is not None: end = int(end) - return self.backend.current_playlist.playlist.mpd_format(start, end) + return self.backend.current_playlist.mpd_format(start, end) @handle_pattern(r'^playlistsearch "(?P[^"]+)" "(?P[^"]+)"$') def _current_playlist_playlistsearch(self, tag, needle): @@ -495,7 +494,7 @@ class MpdFrontend(object): """ # XXX Naive implementation that returns all tracks as changed if int(version) < self.backend.current_playlist.version: - return self.backend.current_playlist.playlist.mpd_format() + return self.backend.current_playlist.mpd_format() @handle_pattern(r'^plchangesposid "(?P\d+)"$') def _current_playlist_plchangesposid(self, version): @@ -515,7 +514,7 @@ class MpdFrontend(object): if int(version) != self.backend.current_playlist.version: result = [] for position, track in enumerate( - self.backend.current_playlist.playlist.tracks): + self.backend.current_playlist.tracks): result.append((u'cpos', position)) result.append((u'Id', track.id)) return result @@ -548,15 +547,14 @@ class MpdFrontend(object): """ songpos1 = int(songpos1) songpos2 = int(songpos2) - playlist = self.backend.current_playlist.playlist - tracks = playlist.tracks + tracks = self.backend.current_playlist.tracks song1 = tracks[songpos1] song2 = tracks[songpos2] del tracks[songpos1] tracks.insert(songpos1, song2) del tracks[songpos2] tracks.insert(songpos2, song1) - self.backend.current_playlist.load(playlist.with_(tracks=tracks)) + self.backend.current_playlist.load(tracks) @handle_pattern(r'^swapid "(?P\d+)" "(?P\d+)"$') def _current_playlist_swapid(self, songid1, songid2): @@ -571,8 +569,8 @@ class MpdFrontend(object): songid2 = int(songid2) song1 = self.backend.current_playlist.get(id=songid1) song2 = self.backend.current_playlist.get(id=songid2) - songpos1 = self.backend.current_playlist.playlist.tracks.index(song1) - songpos2 = self.backend.current_playlist.playlist.tracks.index(song2) + songpos1 = self.backend.current_playlist.tracks.index(song1) + songpos2 = self.backend.current_playlist.tracks.index(song2) self._current_playlist_swap(songpos1, songpos2) @handle_pattern(r'^$') @@ -897,7 +895,7 @@ class MpdFrontend(object): songid = int(songid) try: if songid == -1: - track = self.backend.current_playlist.playlist.tracks[0] + track = self.backend.current_playlist.tracks[0] else: track = self.backend.current_playlist.get(id=songid) return self.backend.playback.play(track) @@ -921,9 +919,9 @@ class MpdFrontend(object): songpos = int(songpos) try: if songpos == -1: - track = self.backend.current_playlist.playlist.tracks[0] + track = self.backend.current_playlist.tracks[0] else: - track = self.backend.current_playlist.playlist.tracks[songpos] + track = self.backend.current_playlist.tracks[songpos] return self.backend.playback.play(track) except IndexError: raise MpdArgError(u'Bad song index', command=u'play') @@ -1332,7 +1330,7 @@ class MpdFrontend(object): return 0 def __status_status_playlist_length(self): - return self.backend.current_playlist.playlist.length + return len(self.backend.current_playlist.tracks) def __status_status_playlist_version(self): return self.backend.current_playlist.version @@ -1537,8 +1535,7 @@ class MpdFrontend(object): """ matches = self.backend.stored_playlists.search(name) if matches: - self.backend.current_playlist.load(matches[0]) - self.backend.playback.new_playlist_loaded_callback() # FIXME not needed? + self.backend.current_playlist.load(matches[0].tracks) @handle_pattern(r'^playlistadd "(?P[^"]+)" "(?P[^"]+)"$') def _stored_playlist_playlistadd(self, name, uri): diff --git a/tests/backends/base.py b/tests/backends/base.py index 129ac4ff..6ecdb608 100644 --- a/tests/backends/base.py +++ b/tests/backends/base.py @@ -44,12 +44,12 @@ class BaseCurrentPlaylistControllerTest(object): def test_add(self): for track in self.tracks: self.controller.add(track) - self.assertEqual(track, self.controller.playlist.tracks[-1]) + self.assertEqual(track, self.controller.tracks[-1]) def test_add_at_position(self): for track in self.tracks[:-1]: self.controller.add(track, 0) - self.assertEqual(track, self.controller.playlist.tracks[0]) + self.assertEqual(track, self.controller.tracks[0]) @populate_playlist def test_add_at_position_outside_of_playlist(self): @@ -58,12 +58,12 @@ class BaseCurrentPlaylistControllerTest(object): @populate_playlist def test_add_sets_id_property(self): - for track in self.controller.playlist.tracks: + for track in self.controller.tracks: self.assertNotEqual(None, track.id) @populate_playlist def test_get_by_id(self): - track = self.controller.playlist.tracks[1] + track = self.controller.tracks[1] self.assertEqual(track, self.controller.get(id=track.id)) @populate_playlist @@ -72,7 +72,7 @@ class BaseCurrentPlaylistControllerTest(object): @populate_playlist def test_get_by_uri(self): - track = self.controller.playlist.tracks[1] + track = self.controller.tracks[1] self.assertEqual(track, self.controller.get(uri=track.uri)) @populate_playlist @@ -83,11 +83,11 @@ class BaseCurrentPlaylistControllerTest(object): @populate_playlist def test_clear(self): self.controller.clear() - self.assertEqual(len(self.controller.playlist.tracks), 0) + self.assertEqual(len(self.controller.tracks), 0) def test_clear_empty_playlist(self): self.controller.clear() - self.assertEqual(len(self.controller.playlist.tracks), 0) + self.assertEqual(len(self.controller.tracks), 0) @populate_playlist def test_clear_when_playing(self): @@ -97,21 +97,19 @@ class BaseCurrentPlaylistControllerTest(object): self.assertEqual(self.playback.state, self.playback.STOPPED) def test_load(self): - new_playlist = Playlist() - self.assertNotEqual(id(new_playlist), id(self.controller.playlist)) - self.controller.load(new_playlist) - # FIXME how do we test this without going into internals? - self.assertEqual(new_playlist, self.controller._playlist) + tracks = [] + self.assertNotEqual(id(tracks), id(self.controller.tracks)) + self.controller.load(tracks) + self.assertEqual(tracks, self.controller.tracks) def test_get_by_id_returns_unique_match(self): track = Track(id=1) - self.controller.playlist = Playlist( - tracks=[Track(id=13), track, Track(id=17)]) + self.controller.load([Track(id=13), track, Track(id=17)]) self.assertEqual(track, self.controller.get(id=1)) def test_get_by_id_raises_error_if_multiple_matches(self): track = Track(id=1) - self.controller.playlist = Playlist(tracks=[Track(id=13), track, track]) + self.controller.load([Track(id=13), track, track]) try: self.controller.get(id=1) self.fail(u'Should raise LookupError if multiple matches') @@ -128,14 +126,12 @@ class BaseCurrentPlaylistControllerTest(object): def test_get_by_uri_returns_unique_match(self): track = Track(uri='a') - self.controller.playlist = Playlist( - tracks=[Track(uri='z'), track, Track(uri='y')]) + self.controller.load([Track(uri='z'), track, Track(uri='y')]) self.assertEqual(track, self.controller.get(uri='a')) def test_get_by_uri_raises_error_if_multiple_matches(self): track = Track(uri='a') - self.controller.playlist = Playlist( - tracks=[Track(uri='z'), track, track]) + self.controller.load([Track(uri='z'), track, track]) try: self.controller.get(uri='a') self.fail(u'Should raise LookupError if multiple matches') @@ -155,7 +151,7 @@ class BaseCurrentPlaylistControllerTest(object): track1 = Track(id=1, uri='a') track2 = Track(id=1, uri='b') track3 = Track(id=2, uri='b') - self.controller.playlist = Playlist(tracks=[track1, track2, track3]) + self.controller.load([track1, track2, track3]) self.assertEqual(track1, self.controller.get(id=1, uri='a')) self.assertEqual(track2, self.controller.get(id=1, uri='b')) self.assertEqual(track3, self.controller.get(id=2, uri='b')) @@ -164,14 +160,13 @@ class BaseCurrentPlaylistControllerTest(object): track1 = Track(id=1) track2 = Track(uri='b') track3 = Track(id=2) - self.controller.playlist = Playlist(tracks=[track1, track2, track3]) + self.controller.load([track1, track2, track3]) self.assertEqual(track1, self.controller.get(id=1)) @populate_playlist def test_load_replaces_playlist(self): - self.backend.current_playlist.load(Playlist()) - tracks = self.backend.current_playlist.playlist.tracks - self.assertEqual(len(tracks), 0) + self.backend.current_playlist.load([]) + self.assertEqual(len(self.backend.current_playlist.tracks), 0) def test_load_does_not_reset_version(self): version = self.controller.version @@ -181,20 +176,20 @@ class BaseCurrentPlaylistControllerTest(object): @populate_playlist def test_load_preserves_playing_state(self): - tracks = self.controller.playlist.tracks + tracks = self.controller.tracks playback = self.playback self.playback.play() - self.controller.load(Playlist(tracks=[tracks[1]])) + self.controller.load([tracks[1]]) self.assertEqual(playback.state, playback.PLAYING) self.assertEqual(tracks[1], self.playback.current_track) @populate_playlist def test_load_preserves_stopped_state(self): - tracks = self.controller.playlist.tracks + tracks = self.controller.tracks playback = self.playback - self.controller.load(Playlist(tracks=[tracks[2]])) + self.controller.load([tracks[2]]) self.assertEqual(playback.state, playback.STOPPED) self.assertEqual(None, self.playback.current_track) @@ -202,32 +197,32 @@ class BaseCurrentPlaylistControllerTest(object): def test_move_single(self): self.controller.move(0, 0, 2) - tracks = self.controller.playlist.tracks + tracks = self.controller.tracks self.assertEqual(tracks[2], self.tracks[0]) @populate_playlist def test_move_group(self): self.controller.move(0, 2, 1) - tracks = self.controller.playlist.tracks + tracks = self.controller.tracks self.assertEqual(tracks[1], self.tracks[0]) self.assertEqual(tracks[2], self.tracks[1]) @populate_playlist def test_moving_track_outside_of_playlist(self): - tracks = len(self.controller.playlist.tracks) + tracks = len(self.controller.tracks) test = lambda: self.controller.move(0, 0, tracks+5) self.assertRaises(AssertionError, test) @populate_playlist def test_move_group_outside_of_playlist(self): - tracks = len(self.controller.playlist.tracks) + tracks = len(self.controller.tracks) test = lambda: self.controller.move(0, 2, tracks+5) self.assertRaises(AssertionError, test) @populate_playlist def test_move_group_out_of_range(self): - tracks = len(self.controller.playlist.tracks) + tracks = len(self.controller.tracks) test = lambda: self.controller.move(tracks+2, tracks+3, 0) self.assertRaises(AssertionError, test) @@ -236,19 +231,18 @@ class BaseCurrentPlaylistControllerTest(object): test = lambda: self.controller.move(2, 1, 0) self.assertRaises(AssertionError, test) - def test_playlist_attribute_is_immutable(self): - playlist1 = self.controller.playlist - playlist2 = self.controller.playlist - - self.assertNotEqual(id(playlist1), id(playlist2)) + def test_tracks_attribute_is_immutable(self): + tracks1 = self.controller.tracks + tracks2 = self.controller.tracks + self.assertNotEqual(id(tracks1), id(tracks2)) @populate_playlist def test_remove(self): - track1 = self.controller.playlist.tracks[1] - track2 = self.controller.playlist.tracks[2] + track1 = self.controller.tracks[1] + track2 = self.controller.tracks[2] self.controller.remove(track1) - self.assert_(track1 not in self.controller.playlist.tracks) - self.assertEqual(track2, self.controller.playlist.tracks[1]) + self.assert_(track1 not in self.controller.tracks) + self.assertEqual(track2, self.controller.tracks[1]) @populate_playlist def test_removing_track_that_does_not_exist(self): @@ -264,7 +258,7 @@ class BaseCurrentPlaylistControllerTest(object): random.seed(1) self.controller.shuffle() - shuffled_tracks = self.controller.playlist.tracks + shuffled_tracks = self.controller.tracks self.assertNotEqual(self.tracks, shuffled_tracks) self.assertEqual(set(self.tracks), set(shuffled_tracks)) @@ -274,7 +268,7 @@ class BaseCurrentPlaylistControllerTest(object): random.seed(1) self.controller.shuffle(1, 3) - shuffled_tracks = self.controller.playlist.tracks + shuffled_tracks = self.controller.tracks self.assertNotEqual(self.tracks, shuffled_tracks) self.assertEqual(self.tracks[0], shuffled_tracks[0]) @@ -287,7 +281,7 @@ class BaseCurrentPlaylistControllerTest(object): @populate_playlist def test_shuffle_superset(self): - tracks = len(self.controller.playlist.tracks) + tracks = len(self.controller.tracks) test = lambda: self.controller.shuffle(1, tracks+5) self.assertRaises(AssertionError, test) @@ -296,7 +290,7 @@ class BaseCurrentPlaylistControllerTest(object): random.seed(1) self.controller.shuffle(1) - shuffled_tracks = self.controller.playlist.tracks + shuffled_tracks = self.controller.tracks self.assertNotEqual(self.tracks, shuffled_tracks) self.assertEqual(self.tracks[0], shuffled_tracks[0]) @@ -304,8 +298,8 @@ class BaseCurrentPlaylistControllerTest(object): def test_version(self): version = self.controller.version - self.controller.playlist = Playlist() - self.assertEqual(version+1, self.controller.version) + self.controller.tracks = [] + self.assertEqual(self.controller.version, version + 1) class BasePlaybackControllerTest(object): @@ -639,26 +633,23 @@ class BasePlaybackControllerTest(object): @populate_playlist def test_new_playlist_loaded_callback_when_playing(self): - playlist = Playlist(tracks=[self.tracks[2]]) self.playback.play() - self.backend.current_playlist.load(playlist) + self.backend.current_playlist.load([self.tracks[2]]) self.assertEqual(self.playback.state, self.playback.PLAYING) self.assertEqual(self.playback.current_track, self.tracks[2]) @populate_playlist def test_new_playlist_loaded_callback_when_stopped(self): - playlist = Playlist(tracks=[self.tracks[2]]) - self.backend.current_playlist.load(playlist) + self.backend.current_playlist.load([self.tracks[2]]) self.assertEqual(self.playback.state, self.playback.STOPPED) self.assertEqual(self.playback.current_track, None) self.assertEqual(self.playback.next_track, self.tracks[2]) @populate_playlist def test_new_playlist_loaded_callback_when_paused(self): - playlist = Playlist(tracks=[self.tracks[2]]) self.playback.play() self.playback.pause() - self.backend.current_playlist.load(playlist) + self.backend.current_playlist.load([self.tracks[2]]) self.assertEqual(self.playback.state, self.playback.STOPPED) self.assertEqual(self.playback.current_track, None) self.assertEqual(self.playback.next_track, self.tracks[2]) @@ -735,7 +726,7 @@ class BasePlaybackControllerTest(object): @populate_playlist def test_seek_when_playing(self): - length = self.backend.current_playlist.playlist.tracks[0].length + length = self.backend.current_playlist.tracks[0].length self.playback.play() self.playback.seek(length - 1000) position = self.playback.time_position @@ -743,7 +734,7 @@ class BasePlaybackControllerTest(object): @populate_playlist def test_seek_when_paused(self): - length = self.backend.current_playlist.playlist.tracks[0].length + length = self.backend.current_playlist.tracks[0].length self.playback.play() self.playback.pause() self.playback.seek(length - 1000) @@ -842,16 +833,15 @@ class BasePlaybackControllerTest(object): self.playback.consume = True self.playback.play() self.playback.next() - tracks = self.backend.current_playlist.playlist.tracks - self.assert_(self.tracks[0] not in tracks) + self.assert_(self.tracks[0] not in self.backend.current_playlist.tracks) @populate_playlist def test_playlist_is_empty_after_all_tracks_are_played_with_consume(self): self.playback.consume = True self.playback.play() - for i in range(len(self.backend.current_playlist.playlist.tracks)): + for i in range(len(self.backend.current_playlist.tracks)): self.playback.next() - self.assertEqual(len(self.backend.current_playlist.playlist.tracks), 0) + self.assertEqual(len(self.backend.current_playlist.tracks), 0) @populate_playlist def test_play_with_random(self): @@ -932,7 +922,7 @@ class BasePlaybackControllerTest(object): random.seed(1) self.playback.random = True self.assertEqual(self.playback.next_track, self.tracks[2]) - self.backend.current_playlist.load(Playlist(tracks=self.tracks[:1])) + self.backend.current_playlist.load(self.tracks[:1]) self.assertEqual(self.playback.next_track, self.tracks[0]) @populate_playlist @@ -946,8 +936,7 @@ class BasePlaybackControllerTest(object): self.playback.next() def test_playing_track_with_invalid_uri(self): - playlist = Playlist(tracks=[Track(uri='foobar')]) - self.backend.current_playlist.load(playlist) + self.backend.current_playlist.load([Track(uri='foobar')]) self.playback.play() self.assertEqual(self.playback.state, self.playback.STOPPED) diff --git a/tests/mpd/frontend_test.py b/tests/mpd/frontend_test.py index a6faab5d..312986f7 100644 --- a/tests/mpd/frontend_test.py +++ b/tests/mpd/frontend_test.py @@ -118,7 +118,7 @@ class StatusHandlerTest(unittest.TestCase): def test_currentsong(self): track = Track() - self.b.current_playlist.playlist = Playlist(tracks=[track]) + self.b.current_playlist.load([track]) self.b.playback.current_track = track result = self.h.handle_request(u'currentsong') self.assert_(u'file: ' in result) @@ -258,7 +258,7 @@ class StatusHandlerTest(unittest.TestCase): def test_status_method_when_playlist_loaded_contains_song(self): track = Track() - self.b.current_playlist.load(Playlist(tracks=[track])) + self.b.current_playlist.load([track]) self.b.playback.current_track = track result = dict(self.h._status_status()) self.assert_('song' in result) @@ -266,7 +266,7 @@ class StatusHandlerTest(unittest.TestCase): def test_status_method_when_playlist_loaded_contains_pos_as_songid(self): track = Track() - self.b.current_playlist.load(Playlist(tracks=[track])) + self.b.current_playlist.load([track]) self.b.playback.current_track = track result = dict(self.h._status_status()) self.assert_('songid' in result) @@ -274,7 +274,7 @@ class StatusHandlerTest(unittest.TestCase): def test_status_method_when_playlist_loaded_contains_id_as_songid(self): track = Track(id=1) - self.b.current_playlist.load(Playlist(tracks=[track])) + self.b.current_playlist.load([track]) self.b.playback.current_track = track result = dict(self.h._status_status()) self.assert_('songid' in result) @@ -447,7 +447,7 @@ class PlaybackControlHandlerTest(unittest.TestCase): def test_pause_off(self): track = Track() - self.b.current_playlist.playlist = Playlist(tracks=[track]) + self.b.current_playlist.load([track]) self.b.playback.current_track = track self.h.handle_request(u'play "0"') self.h.handle_request(u'pause "1"') @@ -457,7 +457,7 @@ class PlaybackControlHandlerTest(unittest.TestCase): def test_pause_on(self): track = Track() - self.b.current_playlist.playlist = Playlist(tracks=[track]) + self.b.current_playlist.load([track]) self.b.playback.current_track = track self.h.handle_request(u'play "0"') result = self.h.handle_request(u'pause "1"') @@ -466,7 +466,7 @@ class PlaybackControlHandlerTest(unittest.TestCase): def test_play_without_pos(self): track = Track() - self.b.current_playlist.playlist = Playlist(tracks=[track]) + self.b.current_playlist.load([track]) self.b.playback.current_track = track self.b.playback.state = self.b.playback.PAUSED result = self.h.handle_request(u'play') @@ -474,41 +474,41 @@ class PlaybackControlHandlerTest(unittest.TestCase): self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) def test_play_with_pos(self): - self.b.current_playlist.load(Playlist(tracks=[Track()])) + self.b.current_playlist.load([Track()]) result = self.h.handle_request(u'play "0"') self.assert_(u'OK' in result) self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) def test_play_with_pos_out_of_bounds(self): - self.b.current_playlist.load(Playlist()) + self.b.current_playlist.load([]) result = self.h.handle_request(u'play "0"') self.assertEqual(result[0], u'ACK [2@0] {play} Bad song index') self.assertEqual(self.b.playback.STOPPED, self.b.playback.state) def test_play_minus_one_plays_first_in_playlist(self): track = Track(id=0) - self.b.current_playlist.load(Playlist(tracks=[track])) + self.b.current_playlist.load([track]) result = self.h.handle_request(u'play "-1"') self.assert_(u'OK' in result) self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) self.assertEqual(self.b.playback.current_track, track) def test_playid(self): - self.b.current_playlist.load(Playlist(tracks=[Track(id=0)])) + self.b.current_playlist.load([Track(id=0)]) result = self.h.handle_request(u'playid "0"') self.assert_(u'OK' in result) self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) def test_playid_minus_one_plays_first_in_playlist(self): track = Track(id=0) - self.b.current_playlist.load(Playlist(tracks=[track])) + self.b.current_playlist.load([track]) result = self.h.handle_request(u'playid "-1"') self.assert_(u'OK' in result) self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) self.assertEqual(self.b.playback.current_track, track) def test_playid_which_does_not_exist(self): - self.b.current_playlist.load(Playlist(tracks=[Track(id=0)])) + self.b.current_playlist.load([Track(id=0)]) result = self.h.handle_request(u'playid "1"') self.assertEqual(result[0], u'ACK [50@0] {playid} No such song') @@ -539,12 +539,12 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): def test_add(self): needle = Track(uri='dummy://foo') self.b.library._library = [Track(), Track(), needle, Track()] - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'add "dummy://foo"') - self.assertEqual(self.b.current_playlist.playlist.length, 6) - self.assertEqual(self.b.current_playlist.playlist.tracks[5], needle) + self.assertEqual(len(self.b.current_playlist.tracks), 6) + self.assertEqual(self.b.current_playlist.tracks[5], needle) self.assertEqual(len(result), 1) self.assert_(u'OK' in result) @@ -556,33 +556,33 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): def test_addid_without_songpos(self): needle = Track(uri='dummy://foo', id=137) self.b.library._library = [Track(), Track(), needle, Track()] - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'addid "dummy://foo"') - self.assertEqual(self.b.current_playlist.playlist.length, 6) - self.assertEqual(self.b.current_playlist.playlist.tracks[5], needle) + 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'OK' in result) def test_addid_with_songpos(self): needle = Track(uri='dummy://foo', id=137) self.b.library._library = [Track(), Track(), needle, Track()] - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'addid "dummy://foo" "3"') - self.assertEqual(self.b.current_playlist.playlist.length, 6) - self.assertEqual(self.b.current_playlist.playlist.tracks[3], needle) + 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'OK' in result) def test_addid_with_songpos_out_of_bounds_should_ack(self): needle = Track(uri='dummy://foo', id=137) self.b.library._library = [Track(), Track(), needle, Track()] - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'addid "dummy://foo" "6"') self.assertEqual(result[0], u'ACK [2@0] {addid} Bad song index') @@ -591,118 +591,122 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assertEqual(result[0], u'ACK [50@0] {addid} No such song') def test_clear(self): - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'clear') - self.assertEqual(self.b.current_playlist.playlist.length, 0) + self.assertEqual(len(self.b.current_playlist.tracks), 0) self.assertEqual(self.b.playback.current_track, None) self.assert_(u'OK' in result) def test_delete_songpos(self): - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'delete "2"') - self.assertEqual(self.b.current_playlist.playlist.length, 4) + self.assertEqual(len(self.b.current_playlist.tracks), 4) self.assert_(u'OK' in result) def test_delete_songpos_out_of_bounds(self): - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'delete "5"') - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.assertEqual(len(self.b.current_playlist.tracks), 5) self.assertEqual(result[0], u'ACK [2@0] {delete} Bad song index') def test_delete_open_range(self): - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'delete "1:"') - self.assertEqual(self.b.current_playlist.playlist.length, 1) + self.assertEqual(len(self.b.current_playlist.tracks), 1) self.assert_(u'OK' in result) def test_delete_closed_range(self): - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'delete "1:3"') - self.assertEqual(self.b.current_playlist.playlist.length, 3) + self.assertEqual(len(self.b.current_playlist.tracks), 3) self.assert_(u'OK' in result) def test_delete_range_out_of_bounds(self): - self.b.current_playlist.playlist = Playlist( - tracks=[Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.b.current_playlist.load( + [Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'delete "5:7"') - self.assertEqual(self.b.current_playlist.playlist.length, 5) + self.assertEqual(len(self.b.current_playlist.tracks), 5) self.assertEqual(result[0], u'ACK [2@0] {delete} Bad song index') def test_deleteid(self): - self.b.current_playlist.load(Playlist(tracks=[Track(id=0), Track()])) - self.assertEqual(self.b.current_playlist.playlist.length, 2) + self.b.current_playlist.load([Track(id=0), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 2) result = self.h.handle_request(u'deleteid "0"') - self.assertEqual(self.b.current_playlist.playlist.length, 1) + self.assertEqual(len(self.b.current_playlist.tracks), 1) self.assert_(u'OK' in result) def test_deleteid_does_not_exist(self): - self.b.current_playlist.load(Playlist(tracks=[Track(id=1), Track()])) - self.assertEqual(self.b.current_playlist.playlist.length, 2) + self.b.current_playlist.load([Track(id=1), Track()]) + self.assertEqual(len(self.b.current_playlist.tracks), 2) result = self.h.handle_request(u'deleteid "0"') - self.assertEqual(self.b.current_playlist.playlist.length, 2) + self.assertEqual(len(self.b.current_playlist.tracks), 2) self.assertEqual(result[0], u'ACK [50@0] {deleteid} No such song') def test_move_songpos(self): - self.b.current_playlist.load(Playlist(tracks=[ + self.b.current_playlist.load([ Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f')])) + Track(name='d'), Track(name='e'), Track(name='f'), + ]) result = self.h.handle_request(u'move "1" "0"') - self.assertEqual(self.b.current_playlist.playlist.tracks[0].name, 'b') - self.assertEqual(self.b.current_playlist.playlist.tracks[1].name, 'a') - self.assertEqual(self.b.current_playlist.playlist.tracks[2].name, 'c') - self.assertEqual(self.b.current_playlist.playlist.tracks[3].name, 'd') - self.assertEqual(self.b.current_playlist.playlist.tracks[4].name, 'e') - self.assertEqual(self.b.current_playlist.playlist.tracks[5].name, 'f') + self.assertEqual(self.b.current_playlist.tracks[0].name, 'b') + self.assertEqual(self.b.current_playlist.tracks[1].name, 'a') + self.assertEqual(self.b.current_playlist.tracks[2].name, 'c') + self.assertEqual(self.b.current_playlist.tracks[3].name, 'd') + self.assertEqual(self.b.current_playlist.tracks[4].name, 'e') + self.assertEqual(self.b.current_playlist.tracks[5].name, 'f') self.assert_(u'OK' in result) def test_move_open_range(self): - self.b.current_playlist.load(Playlist(tracks=[ + self.b.current_playlist.load([ Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f')])) + Track(name='d'), Track(name='e'), Track(name='f'), + ]) result = self.h.handle_request(u'move "2:" "0"') - self.assertEqual(self.b.current_playlist.playlist.tracks[0].name, 'c') - self.assertEqual(self.b.current_playlist.playlist.tracks[1].name, 'd') - self.assertEqual(self.b.current_playlist.playlist.tracks[2].name, 'e') - self.assertEqual(self.b.current_playlist.playlist.tracks[3].name, 'f') - self.assertEqual(self.b.current_playlist.playlist.tracks[4].name, 'a') - self.assertEqual(self.b.current_playlist.playlist.tracks[5].name, 'b') + self.assertEqual(self.b.current_playlist.tracks[0].name, 'c') + self.assertEqual(self.b.current_playlist.tracks[1].name, 'd') + self.assertEqual(self.b.current_playlist.tracks[2].name, 'e') + self.assertEqual(self.b.current_playlist.tracks[3].name, 'f') + self.assertEqual(self.b.current_playlist.tracks[4].name, 'a') + self.assertEqual(self.b.current_playlist.tracks[5].name, 'b') self.assert_(u'OK' in result) def test_move_closed_range(self): - self.b.current_playlist.load(Playlist(tracks=[ + self.b.current_playlist.load([ Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f')])) + Track(name='d'), Track(name='e'), Track(name='f'), + ]) result = self.h.handle_request(u'move "1:3" "0"') - self.assertEqual(self.b.current_playlist.playlist.tracks[0].name, 'b') - self.assertEqual(self.b.current_playlist.playlist.tracks[1].name, 'c') - self.assertEqual(self.b.current_playlist.playlist.tracks[2].name, 'a') - self.assertEqual(self.b.current_playlist.playlist.tracks[3].name, 'd') - self.assertEqual(self.b.current_playlist.playlist.tracks[4].name, 'e') - self.assertEqual(self.b.current_playlist.playlist.tracks[5].name, 'f') + self.assertEqual(self.b.current_playlist.tracks[0].name, 'b') + self.assertEqual(self.b.current_playlist.tracks[1].name, 'c') + self.assertEqual(self.b.current_playlist.tracks[2].name, 'a') + self.assertEqual(self.b.current_playlist.tracks[3].name, 'd') + self.assertEqual(self.b.current_playlist.tracks[4].name, 'e') + self.assertEqual(self.b.current_playlist.tracks[5].name, 'f') self.assert_(u'OK' in result) def test_moveid(self): - self.b.current_playlist.load(Playlist(tracks=[ + self.b.current_playlist.load([ Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e', id=137), Track(name='f')])) + Track(name='d'), Track(name='e', id=137), Track(name='f'), + ]) result = self.h.handle_request(u'moveid "137" "2"') - self.assertEqual(self.b.current_playlist.playlist.tracks[0].name, 'a') - self.assertEqual(self.b.current_playlist.playlist.tracks[1].name, 'b') - self.assertEqual(self.b.current_playlist.playlist.tracks[2].name, 'e') - self.assertEqual(self.b.current_playlist.playlist.tracks[3].name, 'c') - self.assertEqual(self.b.current_playlist.playlist.tracks[4].name, 'd') - self.assertEqual(self.b.current_playlist.playlist.tracks[5].name, 'f') + self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') + self.assertEqual(self.b.current_playlist.tracks[1].name, 'b') + self.assertEqual(self.b.current_playlist.tracks[2].name, 'e') + self.assertEqual(self.b.current_playlist.tracks[3].name, 'c') + self.assertEqual(self.b.current_playlist.tracks[4].name, 'd') + self.assertEqual(self.b.current_playlist.tracks[5].name, 'f') self.assert_(u'OK' in result) def test_playlist_returns_same_as_playlistinfo(self): @@ -715,23 +719,26 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assert_(u'ACK [0@0] {} Not implemented' in result) def test_playlistfind_by_filename(self): - result = self.h.handle_request(u'playlistfind "filename" "file:///dev/null"') + result = self.h.handle_request( + u'playlistfind "filename" "file:///dev/null"') self.assert_(u'OK' in result) def test_playlistfind_by_filename_without_quotes(self): - result = self.h.handle_request(u'playlistfind filename "file:///dev/null"') + result = self.h.handle_request( + u'playlistfind filename "file:///dev/null"') self.assert_(u'OK' in result) def test_playlistfind_by_filename_in_current_playlist(self): - self.b.current_playlist.playlist = Playlist(tracks=[ + self.b.current_playlist.load([ Track(uri='file:///exists')]) - result = self.h.handle_request(u'playlistfind filename "file:///exists"') + result = self.h.handle_request( + u'playlistfind filename "file:///exists"') self.assert_(u'file: file:///exists' in result) self.assert_(u'OK' in result) def test_playlistid_without_songid(self): - self.b.current_playlist.load(Playlist( - tracks=[Track(name='a', id=33), Track(name='b', id=38)])) + self.b.current_playlist.load( + [Track(name='a', id=33), Track(name='b', id=38)]) result = self.h.handle_request(u'playlistid') self.assert_(u'Title: a' in result) self.assert_(u'Id: 33' in result) @@ -740,8 +747,8 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assert_(u'OK' in result) def test_playlistid_with_songid(self): - self.b.current_playlist.load(Playlist( - tracks=[Track(name='a', id=33), Track(name='b', id=38)])) + self.b.current_playlist.load( + [Track(name='a', id=33), Track(name='b', id=38)]) result = self.h.handle_request(u'playlistid "38"') self.assert_(u'Title: a' not in result) self.assert_(u'Id: 33' not in result) @@ -750,8 +757,8 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assert_(u'OK' in result) def test_playlistid_with_not_existing_songid_fails(self): - self.b.current_playlist.load(Playlist( - tracks=[Track(name='a', id=33), Track(name='b', id=38)])) + self.b.current_playlist.load( + [Track(name='a', id=33), Track(name='b', id=38)]) result = self.h.handle_request(u'playlistid "25"') self.assertEqual(result[0], u'ACK [50@0] {playlistid} No such song') @@ -785,8 +792,8 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assert_(u'ACK [0@0] {} Not implemented' in result) def test_plchanges(self): - self.b.current_playlist.load(Playlist( - tracks=[Track(name='a'), Track(name='b'), Track(name='c')])) + self.b.current_playlist.load( + [Track(name='a'), Track(name='b'), Track(name='c')]) result = self.h.handle_request(u'plchanges "0"') self.assert_(u'Title: a' in result) self.assert_(u'Title: b' in result) @@ -794,8 +801,8 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assert_(u'OK' in result) def test_plchangesposid(self): - self.b.current_playlist.load(Playlist( - tracks=[Track(id=11), Track(id=12), Track(id=13)])) + self.b.current_playlist.load( + [Track(id=11), Track(id=12), Track(id=13)]) result = self.h.handle_request(u'plchangesposid "0"') self.assert_(u'cpos: 0' in result) self.assert_(u'Id: 11' in result) @@ -806,64 +813,69 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assert_(u'OK' in result) def test_shuffle_without_range(self): - self.b.current_playlist.load(Playlist(tracks=[ + self.b.current_playlist.load([ Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f')])) + Track(name='d'), Track(name='e'), Track(name='f'), + ]) self.assertEqual(self.b.current_playlist.version, 1) result = self.h.handle_request(u'shuffle') self.assertEqual(self.b.current_playlist.version, 2) self.assert_(u'OK' in result) def test_shuffle_with_open_range(self): - self.b.current_playlist.load(Playlist(tracks=[ + self.b.current_playlist.load([ Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f')])) + Track(name='d'), Track(name='e'), Track(name='f'), + ]) self.assertEqual(self.b.current_playlist.version, 1) result = self.h.handle_request(u'shuffle "4:"') self.assertEqual(self.b.current_playlist.version, 2) - self.assertEqual(self.b.current_playlist.playlist.tracks[0].name, 'a') - self.assertEqual(self.b.current_playlist.playlist.tracks[1].name, 'b') - self.assertEqual(self.b.current_playlist.playlist.tracks[2].name, 'c') - self.assertEqual(self.b.current_playlist.playlist.tracks[3].name, 'd') + self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') + self.assertEqual(self.b.current_playlist.tracks[1].name, 'b') + self.assertEqual(self.b.current_playlist.tracks[2].name, 'c') + self.assertEqual(self.b.current_playlist.tracks[3].name, 'd') self.assert_(u'OK' in result) def test_shuffle_with_closed_range(self): - self.b.current_playlist.load(Playlist(tracks=[ + self.b.current_playlist.load([ Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f')])) + Track(name='d'), Track(name='e'), Track(name='f'), + ]) self.assertEqual(self.b.current_playlist.version, 1) result = self.h.handle_request(u'shuffle "1:3"') self.assertEqual(self.b.current_playlist.version, 2) - self.assertEqual(self.b.current_playlist.playlist.tracks[0].name, 'a') - self.assertEqual(self.b.current_playlist.playlist.tracks[3].name, 'd') - self.assertEqual(self.b.current_playlist.playlist.tracks[4].name, 'e') - self.assertEqual(self.b.current_playlist.playlist.tracks[5].name, 'f') + self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') + self.assertEqual(self.b.current_playlist.tracks[3].name, 'd') + self.assertEqual(self.b.current_playlist.tracks[4].name, 'e') + self.assertEqual(self.b.current_playlist.tracks[5].name, 'f') self.assert_(u'OK' in result) def test_swap(self): - self.b.current_playlist.load(Playlist(tracks=[ + self.b.current_playlist.load([ Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f')])) + Track(name='d'), Track(name='e'), Track(name='f'), + ]) result = self.h.handle_request(u'swap "1" "4"') - self.assertEqual(self.b.current_playlist.playlist.tracks[0].name, 'a') - self.assertEqual(self.b.current_playlist.playlist.tracks[1].name, 'e') - self.assertEqual(self.b.current_playlist.playlist.tracks[2].name, 'c') - self.assertEqual(self.b.current_playlist.playlist.tracks[3].name, 'd') - self.assertEqual(self.b.current_playlist.playlist.tracks[4].name, 'b') - self.assertEqual(self.b.current_playlist.playlist.tracks[5].name, 'f') + self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') + self.assertEqual(self.b.current_playlist.tracks[1].name, 'e') + self.assertEqual(self.b.current_playlist.tracks[2].name, 'c') + self.assertEqual(self.b.current_playlist.tracks[3].name, 'd') + self.assertEqual(self.b.current_playlist.tracks[4].name, 'b') + self.assertEqual(self.b.current_playlist.tracks[5].name, 'f') self.assert_(u'OK' in result) def test_swapid(self): - self.b.current_playlist.load(Playlist(tracks=[ + self.b.current_playlist.load([ Track(name='a'), Track(name='b', id=13), Track(name='c'), - Track(name='d'), Track(name='e', id=29), Track(name='f')])) + Track(name='d'), Track(name='e', id=29), Track(name='f'), + ]) result = self.h.handle_request(u'swapid "13" "29"') - self.assertEqual(self.b.current_playlist.playlist.tracks[0].name, 'a') - self.assertEqual(self.b.current_playlist.playlist.tracks[1].name, 'e') - self.assertEqual(self.b.current_playlist.playlist.tracks[2].name, 'c') - self.assertEqual(self.b.current_playlist.playlist.tracks[3].name, 'd') - self.assertEqual(self.b.current_playlist.playlist.tracks[4].name, 'b') - self.assertEqual(self.b.current_playlist.playlist.tracks[5].name, 'f') + self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') + self.assertEqual(self.b.current_playlist.tracks[1].name, 'e') + self.assertEqual(self.b.current_playlist.tracks[2].name, 'c') + self.assertEqual(self.b.current_playlist.tracks[3].name, 'd') + self.assertEqual(self.b.current_playlist.tracks[4].name, 'b') + self.assertEqual(self.b.current_playlist.tracks[5].name, 'f') self.assert_(u'OK' in result) From 5eabc5a423a48c242ab490b8798298086aab6eb1 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 25 Jun 2010 01:24:18 +0200 Subject: [PATCH 02/13] CurrentPlaylistController.remove() takes criterias --- docs/changes.rst | 4 ++++ mopidy/backends/__init__.py | 10 ++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 267df472..a274e37e 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -31,6 +31,10 @@ 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.remove()`` now takes + criterias, just like + :meth:`mopidy.backends.BaseCurrentPlaylistController.get()``, instead of + the track to remove. 0.1.0a2 (2010-06-02) diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index 3983d94a..260f1ddb 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -198,17 +198,19 @@ class BaseCurrentPlaylistController(object): to_position += 1 self.tracks = new_tracks - def remove(self, track): + def remove(self, **criteria): """ Remove the track from the current playlist. - :param track: track to remove + Uses :meth:`get` to lookup the track to remove. + + :param criteria: on or more criteria to match by + :type criteria: dict :type track: :class:`mopidy.models.Track` """ + track = self.get(**criteria) tracks = self.tracks - assert track in tracks, 'track must be in playlist' - position = tracks.index(track) del tracks[position] self.tracks = tracks From 6ae2ce5772e1c1a8632a793527bd0735f394af14 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 25 Jun 2010 01:24:36 +0200 Subject: [PATCH 03/13] MPD: Update MpdFrontend to use remove() with criterias --- mopidy/mpd/frontend.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/mopidy/mpd/frontend.py b/mopidy/mpd/frontend.py index 3259306e..a675b31a 100644 --- a/mopidy/mpd/frontend.py +++ b/mopidy/mpd/frontend.py @@ -294,7 +294,7 @@ class MpdFrontend(object): if not tracks: raise MpdArgError(u'Bad song index', command=u'delete') for track in tracks: - self.backend.current_playlist.remove(track) + self.backend.current_playlist.remove(cpid=track.cpid) @handle_pattern(r'^delete "(?P\d+)"$') def _current_playlist_delete_songpos(self, songpos): @@ -302,7 +302,7 @@ class MpdFrontend(object): try: songpos = int(songpos) track = self.backend.current_playlist.tracks[songpos] - self.backend.current_playlist.remove(track) + self.backend.current_playlist.remove(id=track.id) except IndexError: raise MpdArgError(u'Bad song index', command=u'delete') @@ -315,10 +315,9 @@ class MpdFrontend(object): Deletes the song ``SONGID`` from the playlist """ - songid = int(songid) try: - track = self.backend.current_playlist.get(id=songid) - return self.backend.current_playlist.remove(track) + songid = int(songid) + return self.backend.current_playlist.remove(id=songid) except LookupError: raise MpdNoExistError(u'No such song', command=u'deleteid') From 93b606af3e92dfcebc0ded44b1b13595cc268c05 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 30 Jun 2010 00:23:28 +0200 Subject: [PATCH 04/13] Fix tests broken by the two previous cherry-picked commits --- mopidy/backends/__init__.py | 2 +- mopidy/mpd/frontend.py | 2 +- tests/backends/base.py | 10 +++++----- tests/mpd/frontend_test.py | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index 260f1ddb..602777ac 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -536,7 +536,7 @@ class BasePlaybackController(object): # FIXME handle in play aswell? if self.consume: - self.backend.current_playlist.remove(original_track) + self.backend.current_playlist.remove(id=original_track.id) if self.random and self.current_track in self._shuffled: self._shuffled.remove(self.current_track) diff --git a/mopidy/mpd/frontend.py b/mopidy/mpd/frontend.py index a675b31a..c687ad66 100644 --- a/mopidy/mpd/frontend.py +++ b/mopidy/mpd/frontend.py @@ -294,7 +294,7 @@ class MpdFrontend(object): if not tracks: raise MpdArgError(u'Bad song index', command=u'delete') for track in tracks: - self.backend.current_playlist.remove(cpid=track.cpid) + self.backend.current_playlist.remove(id=track.id) @handle_pattern(r'^delete "(?P\d+)"$') def _current_playlist_delete_songpos(self, songpos): diff --git a/tests/backends/base.py b/tests/backends/base.py index 6ecdb608..02f9e84c 100644 --- a/tests/backends/base.py +++ b/tests/backends/base.py @@ -240,18 +240,18 @@ class BaseCurrentPlaylistControllerTest(object): def test_remove(self): track1 = self.controller.tracks[1] track2 = self.controller.tracks[2] - self.controller.remove(track1) + self.controller.remove(id=track1.id) self.assert_(track1 not in self.controller.tracks) self.assertEqual(track2, self.controller.tracks[1]) @populate_playlist def test_removing_track_that_does_not_exist(self): - test = lambda: self.controller.remove(Track()) - self.assertRaises(AssertionError, test) + test = lambda: self.controller.remove(id=12345) + self.assertRaises(LookupError, test) def test_removing_from_empty_playlist(self): - test = lambda: self.controller.remove(Track()) - self.assertRaises(AssertionError, test) + test = lambda: self.controller.remove(id=12345) + self.assertRaises(LookupError, test) @populate_playlist def test_shuffle(self): diff --git a/tests/mpd/frontend_test.py b/tests/mpd/frontend_test.py index 312986f7..69a6bbdc 100644 --- a/tests/mpd/frontend_test.py +++ b/tests/mpd/frontend_test.py @@ -601,7 +601,7 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): def test_delete_songpos(self): self.b.current_playlist.load( - [Track(), Track(), Track(), Track(), Track()]) + [Track(id=1), Track(id=2), Track(id=3), Track(id=4), Track(id=5)]) self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'delete "2"') self.assertEqual(len(self.b.current_playlist.tracks), 4) @@ -617,7 +617,7 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): def test_delete_open_range(self): self.b.current_playlist.load( - [Track(), Track(), Track(), Track(), Track()]) + [Track(id=1), Track(id=2), Track(id=3), Track(id=4), Track(id=5)]) self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'delete "1:"') self.assertEqual(len(self.b.current_playlist.tracks), 1) @@ -625,7 +625,7 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): def test_delete_closed_range(self): self.b.current_playlist.load( - [Track(), Track(), Track(), Track(), Track()]) + [Track(id=1), Track(id=2), Track(id=3), Track(id=4), Track(id=5)]) self.assertEqual(len(self.b.current_playlist.tracks), 5) result = self.h.handle_request(u'delete "1:3"') self.assertEqual(len(self.b.current_playlist.tracks), 3) From 1e0a5e5bb3de8221b1c1d1ae7278b1ba4ae29ff3 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 30 Jun 2010 00:28:17 +0200 Subject: [PATCH 05/13] CurrentPlaylistController.load() reuse add() logic --- mopidy/backends/__init__.py | 4 +++- tests/backends/base.py | 7 +++---- tests/mpd/frontend_test.py | 12 ++++++------ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index 602777ac..eef0bca0 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -166,7 +166,9 @@ class BaseCurrentPlaylistController(object): :param tracks: tracks to load :type tracks: list of :class:`mopidy.models.Track` """ - self.tracks = tracks + self.tracks = [] + for track in tracks: + self.add(track) self.backend.playback.new_playlist_loaded_callback() def move(self, start, end, to_position): diff --git a/tests/backends/base.py b/tests/backends/base.py index 02f9e84c..c7bebf4b 100644 --- a/tests/backends/base.py +++ b/tests/backends/base.py @@ -170,9 +170,8 @@ class BaseCurrentPlaylistControllerTest(object): def test_load_does_not_reset_version(self): version = self.controller.version - - self.controller.load(Playlist()) - self.assertEqual(self.controller.version, version+1) + self.controller.load([]) + self.assertEqual(self.controller.version, version + 1) @populate_playlist def test_load_preserves_playing_state(self): @@ -608,7 +607,7 @@ class BasePlaybackControllerTest(object): wrapper.called = False self.playback.new_playlist_loaded_callback = wrapper - self.backend.current_playlist.load(Playlist()) + self.backend.current_playlist.load([]) self.assert_(wrapper.called) diff --git a/tests/mpd/frontend_test.py b/tests/mpd/frontend_test.py index 69a6bbdc..06af5519 100644 --- a/tests/mpd/frontend_test.py +++ b/tests/mpd/frontend_test.py @@ -817,9 +817,9 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): Track(name='a'), Track(name='b'), Track(name='c'), Track(name='d'), Track(name='e'), Track(name='f'), ]) - self.assertEqual(self.b.current_playlist.version, 1) + version = self.b.current_playlist.version result = self.h.handle_request(u'shuffle') - self.assertEqual(self.b.current_playlist.version, 2) + self.assert_(version < self.b.current_playlist.version) self.assert_(u'OK' in result) def test_shuffle_with_open_range(self): @@ -827,9 +827,9 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): Track(name='a'), Track(name='b'), Track(name='c'), Track(name='d'), Track(name='e'), Track(name='f'), ]) - self.assertEqual(self.b.current_playlist.version, 1) + version = self.b.current_playlist.version result = self.h.handle_request(u'shuffle "4:"') - self.assertEqual(self.b.current_playlist.version, 2) + self.assert_(version < self.b.current_playlist.version) self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') self.assertEqual(self.b.current_playlist.tracks[1].name, 'b') self.assertEqual(self.b.current_playlist.tracks[2].name, 'c') @@ -841,9 +841,9 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): Track(name='a'), Track(name='b'), Track(name='c'), Track(name='d'), Track(name='e'), Track(name='f'), ]) - self.assertEqual(self.b.current_playlist.version, 1) + version = self.b.current_playlist.version result = self.h.handle_request(u'shuffle "1:3"') - self.assertEqual(self.b.current_playlist.version, 2) + self.assert_(version < self.b.current_playlist.version) self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') self.assertEqual(self.b.current_playlist.tracks[3].name, 'd') self.assertEqual(self.b.current_playlist.tracks[4].name, 'e') From e6843e8b4d8a6bc1047308c5240132829ff86127 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 30 Jun 2010 21:27:23 +0200 Subject: [PATCH 06/13] CurrentPlaylistController.tracks is now read-only --- docs/changes.rst | 6 ++++-- mopidy/backends/__init__.py | 43 +++++++++++++++++++++---------------- tests/backends/base.py | 4 ++-- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 50ac4917..a0a2f723 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -33,10 +33,12 @@ 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.remove()`` now takes + - :meth:`mopidy.backends.BaseCurrentPlaylistController.remove()` now takes criterias, just like - :meth:`mopidy.backends.BaseCurrentPlaylistController.get()``, instead of + :meth:`mopidy.backends.BaseCurrentPlaylistController.get()`, instead of the track to remove. + - :attr:`mopidy.backends.BaseCurrentPlaylistController.tracks` is now + read-only. Use the methods to change its contents. 0.1.0a2 (2010-06-02) diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index eef0bca0..9a298442 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -96,12 +96,23 @@ class BaseCurrentPlaylistController(object): @property def tracks(self): - """List of :class:`mopidy.model.Track` in the current playlist.""" + """ + List of :class:`mopidy.model.Track` in the current playlist. + + Read-only. + """ return copy(self._tracks) - @tracks.setter - def tracks(self, new_tracks): - self._tracks = copy(new_tracks) + def __set_tracks(self, tracks): + self._tracks = copy(tracks) + self.version += 1 + + def __clear_tracks(self): + self._tracks = [] + self.version += 1 + + def __remove_track(self, position): + del self._tracks[position] self.version += 1 def add(self, track, at_position=None): @@ -115,21 +126,19 @@ class BaseCurrentPlaylistController(object): :type at_position: int or :class:`None` """ tracks = self.tracks - - assert at_position <= len(tracks), 'at_position can not be greater' \ - + ' than playlist length' - + assert at_position <= len(tracks), \ + u'at_position can not be greater than playlist length' if at_position is not None: tracks.insert(at_position, track) else: tracks.append(track) - self.tracks = tracks + self.__set_tracks(tracks) def clear(self): """Clear the current playlist.""" self.backend.playback.stop() self.backend.playback.current_track = None - self.tracks = [] + self.__clear_tracks() def get(self, **criteria): """ @@ -166,7 +175,7 @@ class BaseCurrentPlaylistController(object): :param tracks: tracks to load :type tracks: list of :class:`mopidy.models.Track` """ - self.tracks = [] + self.__clear_tracks() for track in tracks: self.add(track) self.backend.playback.new_playlist_loaded_callback() @@ -198,7 +207,7 @@ class BaseCurrentPlaylistController(object): for track in tracks[start:end]: new_tracks.insert(to_position, track) to_position += 1 - self.tracks = new_tracks + self.__set_tracks(new_tracks) def remove(self, **criteria): """ @@ -211,11 +220,8 @@ class BaseCurrentPlaylistController(object): :type track: :class:`mopidy.models.Track` """ track = self.get(**criteria) - tracks = self.tracks - assert track in tracks, 'track must be in playlist' - position = tracks.index(track) - del tracks[position] - self.tracks = tracks + position = self.tracks.index(track) + self.__remove_track(position) def shuffle(self, start=None, end=None): """ @@ -243,7 +249,8 @@ class BaseCurrentPlaylistController(object): shuffled = tracks[start:end] after = tracks[end or len(tracks):] random.shuffle(shuffled) - self.tracks = before + shuffled + after + self.__set_tracks(before + shuffled + after) + self.version += 1 def destroy(self): """Cleanup after component.""" diff --git a/tests/backends/base.py b/tests/backends/base.py index c7bebf4b..d436b7ca 100644 --- a/tests/backends/base.py +++ b/tests/backends/base.py @@ -297,8 +297,8 @@ class BaseCurrentPlaylistControllerTest(object): def test_version(self): version = self.controller.version - self.controller.tracks = [] - self.assertEqual(self.controller.version, version + 1) + self.controller.load([]) + self.assert_(version < self.controller.version) class BasePlaybackControllerTest(object): From 98ed2d8f48314ab097aa8567e0f0a411d5f610bd Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 1 Jul 2010 00:16:30 +0200 Subject: [PATCH 07/13] Backend: Assign CPID to all tracks added to current playlist --- mopidy/backends/__init__.py | 63 ++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index 9a298442..9b647162 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -88,7 +88,7 @@ class BaseCurrentPlaylistController(object): def __init__(self, backend): self.backend = backend - self._tracks = [] + self._cp_tracks = [] def destroy(self): """Cleanup after component.""" @@ -101,19 +101,7 @@ class BaseCurrentPlaylistController(object): Read-only. """ - return copy(self._tracks) - - def __set_tracks(self, tracks): - self._tracks = copy(tracks) - self.version += 1 - - def __clear_tracks(self): - self._tracks = [] - self.version += 1 - - def __remove_track(self, position): - del self._tracks[position] - self.version += 1 + return [t[1] for t in self._cp_tracks] def add(self, track, at_position=None): """ @@ -125,20 +113,20 @@ class BaseCurrentPlaylistController(object): :param at_position: position in current playlist to add track :type at_position: int or :class:`None` """ - tracks = self.tracks - assert at_position <= len(tracks), \ + assert at_position <= len(self._cp_tracks), \ u'at_position can not be greater than playlist length' if at_position is not None: - tracks.insert(at_position, track) + self._cp_tracks.insert(at_position, (self.version, track)) else: - tracks.append(track) - self.__set_tracks(tracks) + self._cp_tracks.append((self.version, track)) + self.version += 1 def clear(self): """Clear the current playlist.""" self.backend.playback.stop() self.backend.playback.current_track = None - self.__clear_tracks() + self._cp_tracks = [] + self.version += 1 def get(self, **criteria): """ @@ -175,7 +163,8 @@ class BaseCurrentPlaylistController(object): :param tracks: tracks to load :type tracks: list of :class:`mopidy.models.Track` """ - self.__clear_tracks() + self._cp_tracks = [] + self.version += 1 for track in tracks: self.add(track) self.backend.playback.new_playlist_loaded_callback() @@ -194,20 +183,22 @@ class BaseCurrentPlaylistController(object): if start == end: end += 1 - tracks = self.tracks + cp_tracks = self._cp_tracks assert start < end, 'start must be smaller than end' assert start >= 0, 'start must be at least zero' - assert end <= len(tracks), 'end can not be larger than playlist length' + assert end <= len(cp_tracks), \ + 'end can not be larger than playlist length' assert to_position >= 0, 'to_position must be at least zero' - assert to_position <= len(tracks), 'to_position can not be larger ' + \ - 'than playlist length' + assert to_position <= len(cp_tracks), \ + 'to_position can not be larger than playlist length' - new_tracks = tracks[:start] + tracks[end:] - for track in tracks[start:end]: - new_tracks.insert(to_position, track) + new_cp_tracks = cp_tracks[:start] + cp_tracks[end:] + for cp_track in cp_tracks[start:end]: + new_cp_tracks.insert(to_position, cp_track) to_position += 1 - self.__set_tracks(new_tracks) + self._cp_tracks = new_cp_tracks + self.version += 1 def remove(self, **criteria): """ @@ -221,7 +212,7 @@ class BaseCurrentPlaylistController(object): """ track = self.get(**criteria) position = self.tracks.index(track) - self.__remove_track(position) + del self._cp_tracks[position] def shuffle(self, start=None, end=None): """ @@ -233,7 +224,7 @@ class BaseCurrentPlaylistController(object): :param end: position after last track to shuffle :type end: int or :class:`None` """ - tracks = self.tracks + cp_tracks = self._cp_tracks if start is not None and end is not None: assert start < end, 'start must be smaller than end' @@ -242,14 +233,14 @@ class BaseCurrentPlaylistController(object): assert start >= 0, 'start must be at least zero' if end is not None: - assert end <= len(tracks), 'end can not be larger than ' + \ + assert end <= len(cp_tracks), 'end can not be larger than ' + \ 'playlist length' - before = tracks[:start or 0] - shuffled = tracks[start:end] - after = tracks[end or len(tracks):] + before = cp_tracks[:start or 0] + shuffled = cp_tracks[start:end] + after = cp_tracks[end or len(cp_tracks):] random.shuffle(shuffled) - self.__set_tracks(before + shuffled + after) + self._cp_tracks = before + shuffled + after self.version += 1 def destroy(self): From dd494107d370dce1e0de0524e05459958edd9f5b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 1 Jul 2010 00:39:50 +0200 Subject: [PATCH 08/13] Extract mpd_format from models to mopidy.mpd.serializers --- mopidy/models.py | 70 +++------------------------------- mopidy/mpd/serializer.py | 71 ++++++++++++++++++++++++++++++++++ tests/models_test.py | 66 -------------------------------- tests/mpd/serializer_test.py | 74 ++++++++++++++++++++++++++++++++++++ 4 files changed, 151 insertions(+), 130 deletions(-) create mode 100644 mopidy/mpd/serializer.py create mode 100644 tests/mpd/serializer_test.py diff --git a/mopidy/models.py b/mopidy/models.py index df1d12a1..09a509dc 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -1,5 +1,7 @@ from copy import copy +from mopidy.mpd import serializer + class ImmutableObject(object): """ Superclass for immutable objects whose fields can only be modified via the @@ -138,43 +140,8 @@ class Track(ImmutableObject): """List of :class:`Artist`. Read-only.""" return list(self._artists) - def mpd_format(self, position=0, search_result=False): - """ - Format track for output to MPD client. - - :param position: track's position in playlist - :type position: integer - :param search_result: format for output in search result - :type search_result: boolean - :rtype: list of two-tuples - """ - result = [ - ('file', self.uri or ''), - ('Time', self.length and (self.length // 1000) or 0), - ('Artist', self.mpd_format_artists()), - ('Title', self.name or ''), - ('Album', self.album and self.album.name or ''), - ('Date', self.date or ''), - ] - if self.album is not None and self.album.num_tracks != 0: - result.append(('Track', '%d/%d' % ( - self.track_no, self.album.num_tracks))) - else: - result.append(('Track', self.track_no)) - if not search_result: - result.append(('Pos', position)) - result.append(('Id', self.id or position)) - return result - - def mpd_format_artists(self): - """ - Format track artists for output to MPD client. - - :rtype: string - """ - artists = list(self._artists) - artists.sort(key=lambda a: a.name) - return u', '.join([a.name for a in artists]) + def mpd_format(self, *args, **kwargs): + return serializer.track_to_mpd_format(self, *args, **kwargs) class Playlist(ImmutableObject): @@ -214,33 +181,8 @@ class Playlist(ImmutableObject): """The number of tracks in the playlist. Read-only.""" return len(self._tracks) - def mpd_format(self, start=0, end=None, search_result=False): - """ - Format playlist for output to MPD client. - - Optionally limit output to the slice ``[start:end]`` of the playlist. - - :param start: position of first track to include in output - :type start: int (positive or negative) - :param end: position after last track to include in output - :type end: int (positive or negative) or :class:`None` for end of list - :rtype: list of lists of two-tuples - """ - if start < 0: - range_start = self.length + start - else: - range_start = start - if end is not None and end < 0: - range_end = self.length - end - elif end is not None and end >= 0: - range_end = end - else: - range_end = self.length - tracks = [] - for track, position in zip(self.tracks[start:end], - range(range_start, range_end)): - tracks.append(track.mpd_format(position, search_result)) - return tracks + def mpd_format(self, *args, **kwargs): + return serializer.playlist_to_mpd_format(self, *args, **kwargs) def with_(self, uri=None, name=None, tracks=None, last_modified=None): """ diff --git a/mopidy/mpd/serializer.py b/mopidy/mpd/serializer.py new file mode 100644 index 00000000..c88ef1c0 --- /dev/null +++ b/mopidy/mpd/serializer.py @@ -0,0 +1,71 @@ +def track_to_mpd_format(track, position=0, search_result=False): + """ + Format track for output to MPD client. + + :param track: the track + :type track: :class:`mopidy.models.Track` + :param position: track's position in playlist + :type position: integer + :param search_result: format for output in search result + :type search_result: boolean + :rtype: list of two-tuples + """ + result = [ + ('file', track.uri or ''), + ('Time', track.length and (track.length // 1000) or 0), + ('Artist', track_artists_to_mpd_format(track)), + ('Title', track.name or ''), + ('Album', track.album and track.album.name or ''), + ('Date', track.date or ''), + ] + if track.album is not None and track.album.num_tracks != 0: + result.append(('Track', '%d/%d' % ( + track.track_no, track.album.num_tracks))) + else: + result.append(('Track', track.track_no)) + if not search_result: + result.append(('Pos', position)) + result.append(('Id', track.id or position)) + return result + +def track_artists_to_mpd_format(track): + """ + Format track artists for output to MPD client. + + :param track: the track + :type track: :class:`mopidy.models.Track` + :rtype: string + """ + artists = track.artists + artists.sort(key=lambda a: a.name) + return u', '.join([a.name for a in artists]) + +def playlist_to_mpd_format(playlist, start=0, end=None, search_result=False): + """ + Format playlist for output to MPD client. + + Optionally limit output to the slice ``[start:end]`` of the playlist. + + :param playlist: the playlist + :type playlist: :class:`mopidy.models.Playlist` + :param start: position of first track to include in output + :type start: int (positive or negative) + :param end: position after last track to include in output + :type end: int (positive or negative) or :class:`None` for end of list + :rtype: list of lists of two-tuples + """ + if start < 0: + range_start = playlist.length + start + else: + range_start = start + if end is not None and end < 0: + range_end = playlist.length - end + elif end is not None and end >= 0: + range_end = end + else: + range_end = playlist.length + tracks = [] + for track, position in zip(playlist.tracks[start:end], + range(range_start, range_end)): + tracks.append(track.mpd_format(position, search_result)) + return tracks diff --git a/tests/models_test.py b/tests/models_test.py index 1bafd792..7fab86f5 100644 --- a/tests/models_test.py +++ b/tests/models_test.py @@ -228,45 +228,6 @@ class TrackTest(unittest.TestCase): self.assertEqual(track.id, track_id) self.assertRaises(AttributeError, setattr, track, 'id', None) - def test_mpd_format_for_empty_track(self): - track = Track() - result = track.mpd_format() - self.assert_(('file', '') in result) - self.assert_(('Time', 0) in result) - self.assert_(('Artist', '') in result) - self.assert_(('Title', '') in result) - self.assert_(('Album', '') in result) - self.assert_(('Track', 0) in result) - self.assert_(('Date', '') in result) - self.assert_(('Pos', 0) in result) - self.assert_(('Id', 0) in result) - - def test_mpd_format_for_nonempty_track(self): - track = Track( - uri=u'a uri', - artists=[Artist(name=u'an artist')], - name=u'a name', - album=Album(name=u'an album', num_tracks=13), - track_no=7, - date=dt.date(1977, 1, 1), - length=137000, - id=122, - ) - result = track.mpd_format(position=9) - self.assert_(('file', 'a uri') in result) - self.assert_(('Time', 137) in result) - self.assert_(('Artist', 'an artist') in result) - self.assert_(('Title', 'a name') in result) - self.assert_(('Album', 'an album') in result) - self.assert_(('Track', '7/13') in result) - self.assert_(('Date', dt.date(1977, 1, 1)) in result) - self.assert_(('Pos', 9) in result) - self.assert_(('Id', 122) in result) - - def test_mpd_format_artists(self): - track = Track(artists=[Artist(name=u'ABBA'), Artist(name=u'Beatles')]) - self.assertEqual(track.mpd_format_artists(), u'ABBA, Beatles') - def test_invalid_kwarg(self): test = lambda: Track(foo='baz') self.assertRaises(TypeError, test) @@ -450,33 +411,6 @@ class PlaylistTest(unittest.TestCase): self.assertRaises(AttributeError, setattr, playlist, 'last_modified', None) - def test_mpd_format(self): - playlist = Playlist(tracks=[ - Track(track_no=1), Track(track_no=2), Track(track_no=3)]) - result = playlist.mpd_format() - self.assertEqual(len(result), 3) - - def test_mpd_format_with_range(self): - playlist = Playlist(tracks=[ - Track(track_no=1), Track(track_no=2), Track(track_no=3)]) - result = playlist.mpd_format(1, 2) - self.assertEqual(len(result), 1) - self.assertEqual(dict(result[0])['Track'], 2) - - def test_mpd_format_with_negative_start_and_no_end(self): - playlist = Playlist(tracks=[ - Track(track_no=1), Track(track_no=2), Track(track_no=3)]) - result = playlist.mpd_format(-1, None) - self.assertEqual(len(result), 1) - self.assertEqual(dict(result[0])['Track'], 3) - - def test_mpd_format_with_negative_start_and_end(self): - playlist = Playlist(tracks=[ - Track(track_no=1), Track(track_no=2), Track(track_no=3)]) - result = playlist.mpd_format(-2, -1) - self.assertEqual(len(result), 1) - self.assertEqual(dict(result[0])['Track'], 2) - def test_with_new_uri(self): tracks = [Track()] last_modified = dt.datetime.now() diff --git a/tests/mpd/serializer_test.py b/tests/mpd/serializer_test.py new file mode 100644 index 00000000..1ca34cf3 --- /dev/null +++ b/tests/mpd/serializer_test.py @@ -0,0 +1,74 @@ +import datetime as dt +import unittest + +from mopidy.models import Album, Artist, Playlist, Track +from mopidy.mpd import serializer + +class TrackMpdFormatTest(unittest.TestCase): + def test_mpd_format_for_empty_track(self): + result = serializer.track_to_mpd_format(Track()) + self.assert_(('file', '') in result) + self.assert_(('Time', 0) in result) + self.assert_(('Artist', '') in result) + self.assert_(('Title', '') in result) + self.assert_(('Album', '') in result) + self.assert_(('Track', 0) in result) + self.assert_(('Date', '') in result) + self.assert_(('Pos', 0) in result) + self.assert_(('Id', 0) in result) + + def test_mpd_format_for_nonempty_track(self): + track = Track( + uri=u'a uri', + artists=[Artist(name=u'an artist')], + name=u'a name', + album=Album(name=u'an album', num_tracks=13), + track_no=7, + date=dt.date(1977, 1, 1), + length=137000, + id=122, + ) + result = serializer.track_to_mpd_format(track, position=9) + self.assert_(('file', 'a uri') in result) + self.assert_(('Time', 137) in result) + self.assert_(('Artist', 'an artist') in result) + self.assert_(('Title', 'a name') in result) + self.assert_(('Album', 'an album') in result) + self.assert_(('Track', '7/13') in result) + self.assert_(('Date', dt.date(1977, 1, 1)) in result) + self.assert_(('Pos', 9) in result) + self.assert_(('Id', 122) in result) + + def test_mpd_format_artists(self): + track = Track(artists=[Artist(name=u'ABBA'), Artist(name=u'Beatles')]) + self.assertEqual(serializer.track_artists_to_mpd_format(track), + u'ABBA, Beatles') + + +class PlaylistMpdFormatTest(unittest.TestCase): + def test_mpd_format(self): + playlist = Playlist(tracks=[ + Track(track_no=1), Track(track_no=2), Track(track_no=3)]) + result = serializer.playlist_to_mpd_format(playlist) + self.assertEqual(len(result), 3) + + def test_mpd_format_with_range(self): + playlist = Playlist(tracks=[ + Track(track_no=1), Track(track_no=2), Track(track_no=3)]) + result = serializer.playlist_to_mpd_format(playlist, 1, 2) + self.assertEqual(len(result), 1) + self.assertEqual(dict(result[0])['Track'], 2) + + def test_mpd_format_with_negative_start_and_no_end(self): + playlist = Playlist(tracks=[ + Track(track_no=1), Track(track_no=2), Track(track_no=3)]) + result = serializer.playlist_to_mpd_format(playlist, -1, None) + self.assertEqual(len(result), 1) + self.assertEqual(dict(result[0])['Track'], 3) + + def test_mpd_format_with_negative_start_and_end(self): + playlist = Playlist(tracks=[ + Track(track_no=1), Track(track_no=2), Track(track_no=3)]) + result = serializer.playlist_to_mpd_format(playlist, -2, -1) + self.assertEqual(len(result), 1) + self.assertEqual(dict(result[0])['Track'], 2) From 730ca2648c0968d1e1fb2e6fcb26a0c0058ba37b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 1 Jul 2010 01:02:39 +0200 Subject: [PATCH 09/13] Remove duplicate method --- mopidy/backends/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index 9b08250b..9e0bff7a 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -241,10 +241,6 @@ class BaseCurrentPlaylistController(object): random.shuffle(shuffled) self.playlist = self.playlist.with_(tracks=before+shuffled+after) - def destroy(self): - """Cleanup after component.""" - pass - class BaseLibraryController(object): """ From 454d304953825ceaf8ce48a5ca7a97df52b8384a Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 1 Jul 2010 01:08:01 +0200 Subject: [PATCH 10/13] Extract tracks_to_mpd_format from playlist_to_mpd_format --- mopidy/mpd/serializer.py | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/mopidy/mpd/serializer.py b/mopidy/mpd/serializer.py index c88ef1c0..8ec34152 100644 --- a/mopidy/mpd/serializer.py +++ b/mopidy/mpd/serializer.py @@ -40,14 +40,14 @@ def track_artists_to_mpd_format(track): artists.sort(key=lambda a: a.name) return u', '.join([a.name for a in artists]) -def playlist_to_mpd_format(playlist, start=0, end=None, search_result=False): +def tracks_to_mpd_format(tracks, start=0, end=None, search_result=False): """ - Format playlist for output to MPD client. + Format list of tracks for output to MPD client. - Optionally limit output to the slice ``[start:end]`` of the playlist. + Optionally limit output to the slice ``[start:end]`` of the list. - :param playlist: the playlist - :type playlist: :class:`mopidy.models.Playlist` + :param tracks: the tracks + :type tracks: list of :class:`mopidy.models.Track` :param start: position of first track to include in output :type start: int (positive or negative) :param end: position after last track to include in output @@ -55,17 +55,25 @@ def playlist_to_mpd_format(playlist, start=0, end=None, search_result=False): :rtype: list of lists of two-tuples """ if start < 0: - range_start = playlist.length + start + range_start = len(tracks) + start else: range_start = start if end is not None and end < 0: - range_end = playlist.length - end + range_end = len(tracks) - end elif end is not None and end >= 0: range_end = end else: - range_end = playlist.length - tracks = [] - for track, position in zip(playlist.tracks[start:end], + range_end = len(tracks) + result = [] + for track, position in zip(tracks[start:end], range(range_start, range_end)): - tracks.append(track.mpd_format(position, search_result)) - return tracks + result.append(track.mpd_format(position, search_result)) + return result + +def playlist_to_mpd_format(playlist, *args, **kwargs): + """ + Format playlist for output to MPD client. + + Arguments as for :func:`tracks_to_mpd_format`, except the first one. + """ + return tracks_to_mpd_format(playlist.tracks, *args, **kwargs) From 8c3d3603e28464e9b8f9a5405b940080f03bab69 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 1 Jul 2010 01:54:11 +0200 Subject: [PATCH 11/13] Add lookup by CPID to CurrentPlaylistController.get() --- mopidy/backends/__init__.py | 12 +++++++++--- tests/backends/base.py | 6 ++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index 41bb9d9e..50e87c77 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -136,6 +136,8 @@ class BaseCurrentPlaylistController(object): Examples:: + get(cpid=7) # Returns track with CPID 7 + # (current playlist ID) get(id=1) # Returns track with ID 1 get(uri='xyz') # Returns track with URI 'xyz' get(id=1, uri='xyz') # Returns track with ID 1 and URI 'xyz' @@ -144,11 +146,15 @@ class BaseCurrentPlaylistController(object): :type criteria: dict :rtype: :class:`mopidy.models.Track` """ - matches = self.tracks + matches = self._cp_tracks for (key, value) in criteria.iteritems(): - matches = filter(lambda t: getattr(t, key) == value, matches) + if key == 'cpid': + matches = filter(lambda ct: ct[0] == value, matches) + else: + matches = filter(lambda ct: getattr(ct[1], key) == value, + matches) if len(matches) == 1: - return matches[0] + return matches[0][1] # The track part of the only match criteria_string = ', '.join( ['%s=%s' % (k, v) for (k, v) in criteria.iteritems()]) if len(matches) == 0: diff --git a/tests/backends/base.py b/tests/backends/base.py index d436b7ca..1618a221 100644 --- a/tests/backends/base.py +++ b/tests/backends/base.py @@ -61,6 +61,12 @@ class BaseCurrentPlaylistControllerTest(object): for track in self.controller.tracks: self.assertNotEqual(None, track.id) + @populate_playlist + def test_get_by_cpid(self): + track = self.controller.tracks[1] + cpid = self.controller._cp_tracks[1][0] # XXX Messing in internals + self.assertEqual(track, self.controller.get(cpid=cpid)) + @populate_playlist def test_get_by_id(self): track = self.controller.tracks[1] From 6b72ceec4b8339be5b7fa8cfd4694a43c2c49834 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 1 Jul 2010 01:58:56 +0200 Subject: [PATCH 12/13] MpdFrontend: Rename all occurences of songid to cpid --- mopidy/mpd/frontend.py | 61 +++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/mopidy/mpd/frontend.py b/mopidy/mpd/frontend.py index 15fa4a5c..58345351 100644 --- a/mopidy/mpd/frontend.py +++ b/mopidy/mpd/frontend.py @@ -306,8 +306,8 @@ class MpdFrontend(object): except IndexError: raise MpdArgError(u'Bad song index', command=u'delete') - @handle_pattern(r'^deleteid "(?P\d+)"$') - def _current_playlist_deleteid(self, songid): + @handle_pattern(r'^deleteid "(?P\d+)"$') + def _current_playlist_deleteid(self, cpid): """ *musicpd.org, current playlist section:* @@ -316,8 +316,8 @@ class MpdFrontend(object): Deletes the song ``SONGID`` from the playlist """ try: - songid = int(songid) - return self.backend.current_playlist.remove(id=songid) + cpid = int(cpid) + return self.backend.current_playlist.remove(id=cpid) except LookupError: raise MpdNoExistError(u'No such song', command=u'deleteid') @@ -356,8 +356,8 @@ class MpdFrontend(object): to = int(to) self.backend.current_playlist.move(songpos, songpos + 1, to) - @handle_pattern(r'^moveid "(?P\d+)" "(?P\d+)"$') - def _current_playlist_moveid(self, songid, to): + @handle_pattern(r'^moveid "(?P\d+)" "(?P\d+)"$') + def _current_playlist_moveid(self, cpid, to): """ *musicpd.org, current playlist section:* @@ -367,9 +367,9 @@ class MpdFrontend(object): the playlist. If ``TO`` is negative, it is relative to the current song in the playlist (if there is one). """ - songid = int(songid) + cpid = int(cpid) to = int(to) - track = self.backend.current_playlist.get(id=songid) + track = self.backend.current_playlist.get(id=cpid) position = self.backend.current_playlist.tracks.index(track) self.backend.current_playlist.move(position, position + 1, to) @@ -410,8 +410,8 @@ class MpdFrontend(object): return None raise MpdNotImplemented # TODO - @handle_pattern(r'^playlistid( "(?P\d+)")*$') - def _current_playlist_playlistid(self, songid=None): + @handle_pattern(r'^playlistid( "(?P\d+)")*$') + def _current_playlist_playlistid(self, cpid=None): """ *musicpd.org, current playlist section:* @@ -420,10 +420,10 @@ class MpdFrontend(object): Displays a list of songs in the playlist. ``SONGID`` is optional and specifies a single song to display info for. """ - if songid is not None: + if cpid is not None: try: - songid = int(songid) - track = self.backend.current_playlist.get(id=songid) + cpid = int(cpid) + track = self.backend.current_playlist.get(id=cpid) return track.mpd_format() except LookupError: raise MpdNoExistError(u'No such song', command=u'playlistid') @@ -555,8 +555,8 @@ class MpdFrontend(object): tracks.insert(songpos2, song1) self.backend.current_playlist.load(tracks) - @handle_pattern(r'^swapid "(?P\d+)" "(?P\d+)"$') - def _current_playlist_swapid(self, songid1, songid2): + @handle_pattern(r'^swapid "(?P\d+)" "(?P\d+)"$') + def _current_playlist_swapid(self, cpid1, cpid2): """ *musicpd.org, current playlist section:* @@ -564,13 +564,13 @@ class MpdFrontend(object): Swaps the positions of ``SONG1`` and ``SONG2`` (both song ids). """ - songid1 = int(songid1) - songid2 = int(songid2) - song1 = self.backend.current_playlist.get(id=songid1) - song2 = self.backend.current_playlist.get(id=songid2) - songpos1 = self.backend.current_playlist.tracks.index(song1) - songpos2 = self.backend.current_playlist.tracks.index(song2) - self._current_playlist_swap(songpos1, songpos2) + cpid1 = int(cpid1) + cpid2 = int(cpid2) + track1 = self.backend.current_playlist.get(id=cpid1) + track2 = self.backend.current_playlist.get(id=cpid2) + position1 = self.backend.current_playlist.tracks.index(track1) + position2 = self.backend.current_playlist.tracks.index(track2) + self._current_playlist_swap(position1, position2) @handle_pattern(r'^$') def _empty(self): @@ -893,9 +893,9 @@ class MpdFrontend(object): """ return self.backend.playback.play() - @handle_pattern(r'^playid "(?P\d+)"$') - @handle_pattern(r'^playid "(?P-1)"$') - def _playback_playid(self, songid): + @handle_pattern(r'^playid "(?P\d+)"$') + @handle_pattern(r'^playid "(?P-1)"$') + def _playback_playid(self, cpid): """ *musicpd.org, playback section:* @@ -908,12 +908,12 @@ class MpdFrontend(object): - issues ``playid "-1"`` after playlist replacement to start playback at the first track. """ - songid = int(songid) + cpid = int(cpid) try: - if songid == -1: + if cpid == -1: track = self.backend.current_playlist.tracks[0] else: - track = self.backend.current_playlist.get(id=songid) + track = self.backend.current_playlist.get(id=cpid) return self.backend.playback.play(track) except LookupError: raise MpdNoExistError(u'No such song', command=u'playid') @@ -1056,8 +1056,8 @@ class MpdFrontend(object): """ raise MpdNotImplemented # TODO - @handle_pattern(r'^seekid "(?P\d+)" "(?P\d+)"$') - def _playback_seekid(self, songid, seconds): + @handle_pattern(r'^seekid "(?P\d+)" "(?P\d+)"$') + def _playback_seekid(self, cpid, seconds): """ *musicpd.org, playback section:* @@ -1361,6 +1361,7 @@ class MpdFrontend(object): return int(self.backend.playback.single) def __status_status_songid(self): + # TODO Replace track.id with CPID if self.backend.playback.current_track.id is not None: return self.backend.playback.current_track.id else: From affd1799d6fbf9cf04e00b377e66b015b577a132 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 1 Jul 2010 03:15:39 +0200 Subject: [PATCH 13/13] Use CPID in MPD formats. This makes it possible to handle multiple identical songs in the same playlist. --- mopidy/backends/__init__.py | 47 +++++++++------- mopidy/mpd/frontend.py | 31 ++++++----- mopidy/mpd/serializer.py | 33 +++++------ tests/backends/base.py | 2 + tests/mpd/frontend_test.py | 103 ++++++++++++++++++++++++----------- tests/mpd/serializer_test.py | 19 +------ 6 files changed, 133 insertions(+), 102 deletions(-) diff --git a/mopidy/backends/__init__.py b/mopidy/backends/__init__.py index 50e87c77..d6cc4913 100644 --- a/mopidy/backends/__init__.py +++ b/mopidy/backends/__init__.py @@ -5,6 +5,7 @@ import time from mopidy import settings from mopidy.models import Playlist +from mopidy.mpd import serializer from mopidy.utils import get_class logger = logging.getLogger('mopidy.backends.base') @@ -103,6 +104,23 @@ class BaseCurrentPlaylistController(object): """ return [t[1] for t in self._cp_tracks] + def _get_cp_track(self, **criteria): + matches = self._cp_tracks + for (key, value) in criteria.iteritems(): + if key == 'cpid': + matches = filter(lambda ct: ct[0] == value, matches) + else: + matches = filter(lambda ct: getattr(ct[1], key) == value, + matches) + if len(matches) == 1: + return matches[0] + criteria_string = ', '.join( + ['%s=%s' % (k, v) for (k, v) in criteria.iteritems()]) + if len(matches) == 0: + raise LookupError(u'"%s" match no tracks' % criteria_string) + else: + raise LookupError(u'"%s" match multiple tracks' % criteria_string) + def add(self, track, at_position=None): """ Add the track to the end of, or at the given position in the current @@ -146,21 +164,7 @@ class BaseCurrentPlaylistController(object): :type criteria: dict :rtype: :class:`mopidy.models.Track` """ - matches = self._cp_tracks - for (key, value) in criteria.iteritems(): - if key == 'cpid': - matches = filter(lambda ct: ct[0] == value, matches) - else: - matches = filter(lambda ct: getattr(ct[1], key) == value, - matches) - if len(matches) == 1: - return matches[0][1] # The track part of the only match - criteria_string = ', '.join( - ['%s=%s' % (k, v) for (k, v) in criteria.iteritems()]) - if len(matches) == 0: - raise LookupError(u'"%s" match no tracks' % criteria_string) - else: - raise LookupError(u'"%s" match multiple tracks' % criteria_string) + return self._get_cp_track(**criteria)[1] def load(self, tracks): """ @@ -216,9 +220,10 @@ class BaseCurrentPlaylistController(object): :type criteria: dict :type track: :class:`mopidy.models.Track` """ - track = self.get(**criteria) - position = self.tracks.index(track) + cp_track = self._get_cp_track(**criteria) + position = self._cp_tracks.index(cp_track) del self._cp_tracks[position] + self.version += 1 def shuffle(self, start=None, end=None): """ @@ -250,8 +255,9 @@ class BaseCurrentPlaylistController(object): self.version += 1 def mpd_format(self, *args, **kwargs): - # XXX Lazy workaround to make tests pass while refactoring - return Playlist(tracks=self.tracks).mpd_format(*args, **kwargs) + """Not a part of the generic backend API.""" + kwargs['cpids'] = [ct[0] for ct in self._cp_tracks] + return serializer.tracks_to_mpd_format(self.tracks, *args, **kwargs) class BaseLibraryController(object): @@ -332,6 +338,9 @@ class BasePlaybackController(object): #: Tracks are not removed from the playlist. consume = False + #: The CPID (current playlist ID) of :attr:`current_track`. + current_cpid = 0 # TODO Get the correct CPID + #: The currently playing or selected :class:`mopidy.models.Track`. current_track = None diff --git a/mopidy/mpd/frontend.py b/mopidy/mpd/frontend.py index 58345351..a019f454 100644 --- a/mopidy/mpd/frontend.py +++ b/mopidy/mpd/frontend.py @@ -317,7 +317,7 @@ class MpdFrontend(object): """ try: cpid = int(cpid) - return self.backend.current_playlist.remove(id=cpid) + return self.backend.current_playlist.remove(cpid=cpid) except LookupError: raise MpdNoExistError(u'No such song', command=u'deleteid') @@ -369,7 +369,7 @@ class MpdFrontend(object): """ cpid = int(cpid) to = int(to) - track = self.backend.current_playlist.get(id=cpid) + track = self.backend.current_playlist.get(cpid=cpid) position = self.backend.current_playlist.tracks.index(track) self.backend.current_playlist.move(position, position + 1, to) @@ -423,8 +423,9 @@ class MpdFrontend(object): if cpid is not None: try: cpid = int(cpid) - track = self.backend.current_playlist.get(id=cpid) - return track.mpd_format() + track = self.backend.current_playlist.get(cpid=cpid) + position = self.backend.current_playlist.tracks.index(track) + return track.mpd_format(position=position, cpid=cpid) except LookupError: raise MpdNoExistError(u'No such song', command=u'playlistid') else: @@ -463,8 +464,12 @@ class MpdFrontend(object): if start is None: start = 0 start = int(start) + if not (0 <= start <= len(self.backend.current_playlist.tracks)): + raise MpdArgError(u'Bad song index', command=u'playlistinfo') if end is not None: end = int(end) + if end > len(self.backend.current_playlist.tracks): + end = None return self.backend.current_playlist.mpd_format(start, end) @handle_pattern(r'^playlistsearch "(?P[^"]+)" "(?P[^"]+)"$') @@ -566,8 +571,8 @@ class MpdFrontend(object): """ cpid1 = int(cpid1) cpid2 = int(cpid2) - track1 = self.backend.current_playlist.get(id=cpid1) - track2 = self.backend.current_playlist.get(id=cpid2) + track1 = self.backend.current_playlist.get(cpid=cpid1) + track2 = self.backend.current_playlist.get(cpid=cpid2) position1 = self.backend.current_playlist.tracks.index(track1) position2 = self.backend.current_playlist.tracks.index(track2) self._current_playlist_swap(position1, position2) @@ -618,8 +623,7 @@ class MpdFrontend(object): field = field.lower() if field == u'title': field = u'track' - return self.backend.library.find_exact(field, what).mpd_format( - search_result=True) + return self.backend.library.find_exact(field, what).mpd_format() @handle_pattern(r'^findadd "(?P(album|artist|title))" ' r'"(?P[^"]+)"$') @@ -764,8 +768,7 @@ class MpdFrontend(object): field = field.lower() if field == u'title': field = u'track' - return self.backend.library.search(field, what).mpd_format( - search_result=True) + return self.backend.library.search(field, what).mpd_format() @handle_pattern(r'^update( "(?P[^"]+)")*$') def _music_db_update(self, uri=None, rescan_unmodified_files=False): @@ -913,7 +916,7 @@ class MpdFrontend(object): if cpid == -1: track = self.backend.current_playlist.tracks[0] else: - track = self.backend.current_playlist.get(id=cpid) + track = self.backend.current_playlist.get(cpid=cpid) return self.backend.playback.play(track) except LookupError: raise MpdNoExistError(u'No such song', command=u'playid') @@ -1211,7 +1214,8 @@ class MpdFrontend(object): """ if self.backend.playback.current_track is not None: return self.backend.playback.current_track.mpd_format( - position=self.backend.playback.current_playlist_position) + position=self.backend.playback.current_playlist_position, + cpid=self.backend.playback.current_cpid) @handle_pattern(r'^idle$') @handle_pattern(r'^idle (?P.+)$') @@ -1501,8 +1505,7 @@ class MpdFrontend(object): Album, Artist, Track """ try: - return self.backend.stored_playlists.get(name=name).mpd_format( - search_result=True) + return self.backend.stored_playlists.get(name=name).mpd_format() except LookupError: raise MpdNoExistError( u'No such playlist', command=u'listplaylistinfo') diff --git a/mopidy/mpd/serializer.py b/mopidy/mpd/serializer.py index 8ec34152..b8b6550b 100644 --- a/mopidy/mpd/serializer.py +++ b/mopidy/mpd/serializer.py @@ -1,4 +1,4 @@ -def track_to_mpd_format(track, position=0, search_result=False): +def track_to_mpd_format(track, position=None, cpid=None): """ Format track for output to MPD client. @@ -6,8 +6,8 @@ def track_to_mpd_format(track, position=0, search_result=False): :type track: :class:`mopidy.models.Track` :param position: track's position in playlist :type position: integer - :param search_result: format for output in search result - :type search_result: boolean + :param cpid: track's CPID (current playlist ID) + :type cpid: integer :rtype: list of two-tuples """ result = [ @@ -23,9 +23,9 @@ def track_to_mpd_format(track, position=0, search_result=False): track.track_no, track.album.num_tracks))) else: result.append(('Track', track.track_no)) - if not search_result: + if position is not None and cpid is not None: result.append(('Pos', position)) - result.append(('Id', track.id or position)) + result.append(('Id', cpid)) return result def track_artists_to_mpd_format(track): @@ -40,7 +40,7 @@ def track_artists_to_mpd_format(track): artists.sort(key=lambda a: a.name) return u', '.join([a.name for a in artists]) -def tracks_to_mpd_format(tracks, start=0, end=None, search_result=False): +def tracks_to_mpd_format(tracks, start=0, end=None, cpids=None): """ Format list of tracks for output to MPD client. @@ -54,20 +54,15 @@ def tracks_to_mpd_format(tracks, start=0, end=None, search_result=False): :type end: int (positive or negative) or :class:`None` for end of list :rtype: list of lists of two-tuples """ - if start < 0: - range_start = len(tracks) + start - else: - range_start = start - if end is not None and end < 0: - range_end = len(tracks) - end - elif end is not None and end >= 0: - range_end = end - else: - range_end = len(tracks) + if end is None: + end = len(tracks) + tracks = tracks[start:end] + positions = range(start, end) + cpids = cpids and cpids[start:end] or [None for t in tracks] + assert len(tracks) == len(positions) == len(cpids) result = [] - for track, position in zip(tracks[start:end], - range(range_start, range_end)): - result.append(track.mpd_format(position, search_result)) + for track, position, cpid in zip(tracks, positions, cpids): + result.append(track_to_mpd_format(track, position, cpid)) return result def playlist_to_mpd_format(playlist, *args, **kwargs): diff --git a/tests/backends/base.py b/tests/backends/base.py index 1618a221..aa0f56e5 100644 --- a/tests/backends/base.py +++ b/tests/backends/base.py @@ -245,7 +245,9 @@ class BaseCurrentPlaylistControllerTest(object): def test_remove(self): track1 = self.controller.tracks[1] track2 = self.controller.tracks[2] + version = self.controller.version self.controller.remove(id=track1.id) + self.assert_(version < self.controller.version) self.assert_(track1 not in self.controller.tracks) self.assertEqual(track2, self.controller.tracks[1]) diff --git a/tests/mpd/frontend_test.py b/tests/mpd/frontend_test.py index 447f49a6..ea26b526 100644 --- a/tests/mpd/frontend_test.py +++ b/tests/mpd/frontend_test.py @@ -486,7 +486,7 @@ class PlaybackControlHandlerTest(unittest.TestCase): self.assertEqual(self.b.playback.STOPPED, self.b.playback.state) def test_play_minus_one_plays_first_in_playlist(self): - track = Track(id=0) + track = Track() self.b.current_playlist.load([track]) result = self.h.handle_request(u'play "-1"') self.assert_(u'OK' in result) @@ -494,13 +494,13 @@ class PlaybackControlHandlerTest(unittest.TestCase): self.assertEqual(self.b.playback.current_track, track) def test_playid(self): - self.b.current_playlist.load([Track(id=0)]) - result = self.h.handle_request(u'playid "0"') + self.b.current_playlist.load([Track()]) + result = self.h.handle_request(u'playid "1"') self.assert_(u'OK' in result) self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) def test_playid_minus_one_plays_first_in_playlist(self): - track = Track(id=0) + track = Track() self.b.current_playlist.load([track]) result = self.h.handle_request(u'playid "-1"') self.assert_(u'OK' in result) @@ -508,8 +508,8 @@ class PlaybackControlHandlerTest(unittest.TestCase): self.assertEqual(self.b.playback.current_track, track) def test_playid_which_does_not_exist(self): - self.b.current_playlist.load([Track(id=0)]) - result = self.h.handle_request(u'playid "1"') + self.b.current_playlist.load([Track()]) + result = self.h.handle_request(u'playid "12345"') self.assertEqual(result[0], u'ACK [50@0] {playid} No such song') def test_previous(self): @@ -640,16 +640,16 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assertEqual(result[0], u'ACK [2@0] {delete} Bad song index') def test_deleteid(self): - self.b.current_playlist.load([Track(id=0), Track()]) + self.b.current_playlist.load([Track(), Track()]) self.assertEqual(len(self.b.current_playlist.tracks), 2) - result = self.h.handle_request(u'deleteid "0"') + result = self.h.handle_request(u'deleteid "2"') self.assertEqual(len(self.b.current_playlist.tracks), 1) self.assert_(u'OK' in result) def test_deleteid_does_not_exist(self): - self.b.current_playlist.load([Track(id=1), Track()]) + self.b.current_playlist.load([Track(), Track()]) self.assertEqual(len(self.b.current_playlist.tracks), 2) - result = self.h.handle_request(u'deleteid "0"') + result = self.h.handle_request(u'deleteid "12345"') self.assertEqual(len(self.b.current_playlist.tracks), 2) self.assertEqual(result[0], u'ACK [50@0] {deleteid} No such song') @@ -698,9 +698,9 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): def test_moveid(self): self.b.current_playlist.load([ Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e', id=137), Track(name='f'), + Track(name='d'), Track(name='e'), Track(name='f'), ]) - result = self.h.handle_request(u'moveid "137" "2"') + result = self.h.handle_request(u'moveid "5" "2"') self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') self.assertEqual(self.b.current_playlist.tracks[1].name, 'b') self.assertEqual(self.b.current_playlist.tracks[2].name, 'e') @@ -737,39 +737,52 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assert_(u'OK' in result) def test_playlistid_without_songid(self): - self.b.current_playlist.load( - [Track(name='a', id=33), Track(name='b', id=38)]) + self.b.current_playlist.load([Track(name='a'), Track(name='b')]) result = self.h.handle_request(u'playlistid') self.assert_(u'Title: a' in result) - self.assert_(u'Id: 33' in result) self.assert_(u'Title: b' in result) - self.assert_(u'Id: 38' in result) self.assert_(u'OK' in result) def test_playlistid_with_songid(self): - self.b.current_playlist.load( - [Track(name='a', id=33), Track(name='b', id=38)]) - result = self.h.handle_request(u'playlistid "38"') + self.b.current_playlist.load([Track(name='a'), Track(name='b')]) + result = self.h.handle_request(u'playlistid "2"') self.assert_(u'Title: a' not in result) - self.assert_(u'Id: 33' not in result) + self.assert_(u'Id: 1' not in result) self.assert_(u'Title: b' in result) - self.assert_(u'Id: 38' in result) + self.assert_(u'Id: 2' in result) self.assert_(u'OK' in result) def test_playlistid_with_not_existing_songid_fails(self): - self.b.current_playlist.load( - [Track(name='a', id=33), Track(name='b', id=38)]) + self.b.current_playlist.load([Track(name='a'), Track(name='b')]) result = self.h.handle_request(u'playlistid "25"') self.assertEqual(result[0], u'ACK [50@0] {playlistid} No such song') def test_playlistinfo_without_songpos_or_range(self): - # FIXME testing just ok is not enough + self.b.current_playlist.load([ + Track(name='a'), Track(name='b'), Track(name='c'), + Track(name='d'), Track(name='e'), Track(name='f'), + ]) result = self.h.handle_request(u'playlistinfo') + self.assert_(u'Title: a' in result) + self.assert_(u'Title: b' in result) + self.assert_(u'Title: c' in result) + self.assert_(u'Title: d' in result) + self.assert_(u'Title: e' in result) + self.assert_(u'Title: f' in result) self.assert_(u'OK' in result) def test_playlistinfo_with_songpos(self): - # FIXME testing just ok is not enough - result = self.h.handle_request(u'playlistinfo "5"') + self.b.current_playlist.load([ + Track(name='a'), Track(name='b'), Track(name='c'), + Track(name='d'), Track(name='e'), Track(name='f'), + ]) + result = self.h.handle_request(u'playlistinfo "4"') + self.assert_(u'Title: a' not in result) + self.assert_(u'Title: b' not in result) + self.assert_(u'Title: c' not in result) + self.assert_(u'Title: d' not in result) + self.assert_(u'Title: e' in result) + self.assert_(u'Title: f' not in result) self.assert_(u'OK' in result) def test_playlistinfo_with_negative_songpos_same_as_playlistinfo(self): @@ -778,13 +791,39 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assertEqual(result1, result2) def test_playlistinfo_with_open_range(self): - # FIXME testing just ok is not enough - result = self.h.handle_request(u'playlistinfo "10:"') + self.b.current_playlist.load([ + Track(name='a'), Track(name='b'), Track(name='c'), + Track(name='d'), Track(name='e'), Track(name='f'), + ]) + result = self.h.handle_request(u'playlistinfo "2:"') + self.assert_(u'Title: a' not in result) + self.assert_(u'Title: b' not in result) + self.assert_(u'Title: c' in result) + self.assert_(u'Title: d' in result) + self.assert_(u'Title: e' in result) + self.assert_(u'Title: f' in result) self.assert_(u'OK' in result) def test_playlistinfo_with_closed_range(self): - # FIXME testing just ok is not enough + self.b.current_playlist.load([ + Track(name='a'), Track(name='b'), Track(name='c'), + Track(name='d'), Track(name='e'), Track(name='f'), + ]) + result = self.h.handle_request(u'playlistinfo "2:4"') + self.assert_(u'Title: a' not in result) + self.assert_(u'Title: b' not in result) + self.assert_(u'Title: c' in result) + self.assert_(u'Title: d' in result) + self.assert_(u'Title: e' not in result) + self.assert_(u'Title: f' not in result) + self.assert_(u'OK' in result) + + def test_playlistinfo_with_too_high_start_of_range_returns_arg_error(self): result = self.h.handle_request(u'playlistinfo "10:20"') + self.assert_(u'ACK [2@0] {playlistinfo} Bad song index' in result) + + def test_playlistinfo_with_too_high_end_of_range_returns_ok(self): + result = self.h.handle_request(u'playlistinfo "0:20"') self.assert_(u'OK' in result) def test_playlistsearch(self): @@ -866,10 +905,10 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): def test_swapid(self): self.b.current_playlist.load([ - Track(name='a'), Track(name='b', id=13), Track(name='c'), - Track(name='d'), Track(name='e', id=29), Track(name='f'), + Track(name='a'), Track(name='b'), Track(name='c'), + Track(name='d'), Track(name='e'), Track(name='f'), ]) - result = self.h.handle_request(u'swapid "13" "29"') + result = self.h.handle_request(u'swapid "2" "5"') self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') self.assertEqual(self.b.current_playlist.tracks[1].name, 'e') self.assertEqual(self.b.current_playlist.tracks[2].name, 'c') diff --git a/tests/mpd/serializer_test.py b/tests/mpd/serializer_test.py index 1ca34cf3..38c240c2 100644 --- a/tests/mpd/serializer_test.py +++ b/tests/mpd/serializer_test.py @@ -14,8 +14,6 @@ class TrackMpdFormatTest(unittest.TestCase): self.assert_(('Album', '') in result) self.assert_(('Track', 0) in result) self.assert_(('Date', '') in result) - self.assert_(('Pos', 0) in result) - self.assert_(('Id', 0) in result) def test_mpd_format_for_nonempty_track(self): track = Track( @@ -26,9 +24,8 @@ class TrackMpdFormatTest(unittest.TestCase): track_no=7, date=dt.date(1977, 1, 1), length=137000, - id=122, ) - result = serializer.track_to_mpd_format(track, position=9) + result = serializer.track_to_mpd_format(track, position=9, cpid=122) self.assert_(('file', 'a uri') in result) self.assert_(('Time', 137) in result) self.assert_(('Artist', 'an artist') in result) @@ -58,17 +55,3 @@ class PlaylistMpdFormatTest(unittest.TestCase): result = serializer.playlist_to_mpd_format(playlist, 1, 2) self.assertEqual(len(result), 1) self.assertEqual(dict(result[0])['Track'], 2) - - def test_mpd_format_with_negative_start_and_no_end(self): - playlist = Playlist(tracks=[ - Track(track_no=1), Track(track_no=2), Track(track_no=3)]) - result = serializer.playlist_to_mpd_format(playlist, -1, None) - self.assertEqual(len(result), 1) - self.assertEqual(dict(result[0])['Track'], 3) - - def test_mpd_format_with_negative_start_and_end(self): - playlist = Playlist(tracks=[ - Track(track_no=1), Track(track_no=2), Track(track_no=3)]) - result = serializer.playlist_to_mpd_format(playlist, -2, -1) - self.assertEqual(len(result), 1) - self.assertEqual(dict(result[0])['Track'], 2)