From cda2fbbe965765a1cdf4224fb125b2c70bbe4d66 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 28 Dec 2011 01:55:02 +0100 Subject: [PATCH] Add index() method to CurrentPlaylistController to reduce copying of the playlist --- docs/changes.rst | 7 ++++--- mopidy/backends/base/current_playlist.py | 14 +++++++++++++- mopidy/frontends/mpd/protocol/current_playlist.py | 14 +++++--------- tests/backends/base/current_playlist.py | 14 +++++++++++++- 4 files changed, 35 insertions(+), 14 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index ac2c21bf..5bebd49e 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -18,13 +18,14 @@ v0.7.0 (in development) if you use playlist folders, you will no longer get lots of log messages about bad playlists. +- The MPD command ``playlistinfo`` is now faster, thanks to John Bäckstrand. + - Added the method - :meth:`mopidy.backends.base.CurrentPlaylistController.length()` to reduce the + :meth:`mopidy.backends.base.CurrentPlaylistController.length()` and + :meth:`mopidy.backends.base.CurrentPlaylistController.index()` to reduce the need for copying the entire current playlist from one thread to another. Thanks to John Bäckstrand. -- The MPD command ``playlistinfo`` is now faster, thanks to John Bäckstrand. - v0.6.0 (2011-10-09) =================== diff --git a/mopidy/backends/base/current_playlist.py b/mopidy/backends/base/current_playlist.py index 966456a4..9764ed6a 100644 --- a/mopidy/backends/base/current_playlist.py +++ b/mopidy/backends/base/current_playlist.py @@ -136,6 +136,19 @@ class CurrentPlaylistController(object): else: raise LookupError(u'"%s" match multiple tracks' % criteria_string) + def index(self, cp_track): + """ + Get index of the given (CPID integer, :class:`mopidy.models.Track`) + two-tuple in the current playlist. + + Raises :exc:`ValueError` if not found. + + :param cp_track: track to find the index of + :type cp_track: two-tuple (CPID integer, :class:`mopidy.models.Track`) + :rtype: int + """ + return self._cp_tracks.index(cp_track) + def move(self, start, end, to_position): """ Move the tracks in the slice ``[start:end]`` to ``to_position``. @@ -175,7 +188,6 @@ class CurrentPlaylistController(object): :param criteria: on or more criteria to match by :type criteria: dict - :type track: :class:`mopidy.models.Track` """ cp_track = self.get(**criteria) position = self._cp_tracks.index(cp_track) diff --git a/mopidy/frontends/mpd/protocol/current_playlist.py b/mopidy/frontends/mpd/protocol/current_playlist.py index ca0d1156..9d8c73b2 100644 --- a/mopidy/frontends/mpd/protocol/current_playlist.py +++ b/mopidy/frontends/mpd/protocol/current_playlist.py @@ -158,8 +158,7 @@ def moveid(context, cpid, to): cpid = int(cpid) to = int(to) cp_track = context.backend.current_playlist.get(cpid=cpid).get() - position = context.backend.current_playlist.cp_tracks.get().index( - cp_track) + position = context.backend.current_playlist.index(cp_track).get() context.backend.current_playlist.move(position, position + 1, to) @handle_request(r'^playlist$') @@ -194,8 +193,7 @@ def playlistfind(context, tag, needle): if tag == 'filename': try: cp_track = context.backend.current_playlist.get(uri=needle).get() - position = context.backend.current_playlist.cp_tracks.get().index( - cp_track) + position = context.backend.current_playlist.index(cp_track).get() return track_to_mpd_format(cp_track, position=position) except LookupError: return None @@ -215,8 +213,7 @@ def playlistid(context, cpid=None): try: cpid = int(cpid) cp_track = context.backend.current_playlist.get(cpid=cpid).get() - position = context.backend.current_playlist.cp_tracks.get().index( - cp_track) + position = context.backend.current_playlist.index(cp_track).get() return track_to_mpd_format(cp_track, position=position) except LookupError: raise MpdNoExistError(u'No such song', command=u'playlistid') @@ -375,7 +372,6 @@ def swapid(context, cpid1, cpid2): cpid2 = int(cpid2) cp_track1 = context.backend.current_playlist.get(cpid=cpid1).get() cp_track2 = context.backend.current_playlist.get(cpid=cpid2).get() - cp_tracks = context.backend.current_playlist.cp_tracks.get() - position1 = cp_tracks.index(cp_track1) - position2 = cp_tracks.index(cp_track2) + position1 = context.backend.current_playlist.index(cp_track1).get() + position2 = context.backend.current_playlist.index(cp_track2).get() swap(context, position1, position2) diff --git a/tests/backends/base/current_playlist.py b/tests/backends/base/current_playlist.py index 6d4854a7..2dab625e 100644 --- a/tests/backends/base/current_playlist.py +++ b/tests/backends/base/current_playlist.py @@ -1,7 +1,7 @@ import mock import random -from mopidy.models import Playlist, Track +from mopidy.models import CpTrack, Playlist, Track from mopidy.gstreamer import GStreamer from tests.backends.base import populate_playlist @@ -143,6 +143,18 @@ class CurrentPlaylistControllerTest(object): self.assertEqual(self.playback.state, self.playback.STOPPED) self.assertEqual(self.playback.current_track, None) + def test_index_returns_index_of_track(self): + cp_tracks = [] + for track in self.tracks: + cp_tracks.append(self.controller.add(track)) + self.assertEquals(0, self.controller.index(cp_tracks[0])) + self.assertEquals(1, self.controller.index(cp_tracks[1])) + self.assertEquals(2, self.controller.index(cp_tracks[2])) + + def test_index_raises_value_error_if_item_not_found(self): + test = lambda: self.controller.index(CpTrack(0, Track())) + self.assertRaises(ValueError, test) + @populate_playlist def test_move_single(self): self.controller.move(0, 0, 2)