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)