Replace {tracklist,playlists}.get() with .filter() which always returns a list

This commit is contained in:
Stein Magnus Jodal 2012-11-19 20:45:41 +01:00
parent 3bd9d2096f
commit 32639ea8de
9 changed files with 152 additions and 157 deletions

View File

@ -132,6 +132,15 @@ backends:
:meth:`mopidy.core.LibraryController.search` now returns plain lists of
tracks instead of playlist objects.
- :meth:`mopidy.core.TracklistController.get` has been replaced by
:meth:`mopidy.core.TracklistController.filter`.
- :meth:`mopidy.core.PlaylistsController.get` has been replaced by
:meth:`mopidy.core.PlaylistsController.filter`.
- :meth:`mopidy.core.TracklistController.remove` can now remove multiple
tracks, and returns the tracks it removed.
**Bug fixes**
- :issue:`218`: The MPD commands ``listplaylist`` and ``listplaylistinfo`` now

View File

@ -69,35 +69,25 @@ class PlaylistsController(object):
if backend:
backend.playlists.delete(uri).get()
def get(self, **criteria):
def filter(self, **criteria):
"""
Get playlist by given criterias from the set of playlists.
Raises :exc:`LookupError` if a unique match is not found.
Filter playlists by the given criterias.
Examples::
get(name='a') # Returns track with name 'a'
get(uri='xyz') # Returns track with URI 'xyz'
get(name='a', uri='xyz') # Returns track with name 'a' and URI
# 'xyz'
filter(name='a') # Returns track with name 'a'
filter(uri='xyz') # Returns track with URI 'xyz'
filter(name='a', uri='xyz') # Returns track with name 'a' and URI
# 'xyz'
:param criteria: one or more criteria to match by
:type criteria: dict
:rtype: :class:`mopidy.models.Playlist`
:rtype: list of :class:`mopidy.models.Playlist`
"""
matches = self.playlists
for (key, value) in criteria.iteritems():
matches = filter(lambda p: getattr(p, 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('"%s" match no playlists' % criteria_string)
else:
raise LookupError(
'"%s" match multiple playlists' % criteria_string)
return matches
def lookup(self, uri):
"""

View File

@ -116,22 +116,20 @@ class TracklistController(object):
self._tl_tracks = []
self.version += 1
def get(self, **criteria):
def filter(self, **criteria):
"""
Get track by given criterias from tracklist.
Raises :exc:`LookupError` if a unique match is not found.
Filter the tracklist by the given criterias.
Examples::
get(tlid=7) # Returns track with TLID 7 (tracklist 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'
filter(tlid=7) # Returns track with TLID 7 (tracklist ID)
filter(id=1) # Returns track with ID 1
filter(uri='xyz') # Returns track with URI 'xyz'
filter(id=1, uri='xyz') # Returns track with ID 1 and URI 'xyz'
:param criteria: on or more criteria to match by
:type criteria: dict
:rtype: :class:`mopidy.models.TlTrack`
:rtype: list of :class:`mopidy.models.TlTrack`
"""
matches = self._tl_tracks
for (key, value) in criteria.iteritems():
@ -140,14 +138,7 @@ class TracklistController(object):
else:
matches = filter(
lambda ct: getattr(ct.track, 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('"%s" match no tracks' % criteria_string)
else:
raise LookupError('"%s" match multiple tracks' % criteria_string)
return matches
def index(self, tl_track):
"""
@ -197,20 +188,23 @@ class TracklistController(object):
def remove(self, **criteria):
"""
Remove the track from the tracklist.
Remove the matching tracks from the tracklist.
Uses :meth:`get()` to lookup the track to remove.
Uses :meth:`filter()` to lookup the tracks to remove.
Triggers the :method:`mopidy.core.CoreListener.tracklist_changed`
event.
:param criteria: on or more criteria to match by
:type criteria: dict
:rtype: list of :class:`mopidy.models.TlTrack` that was removed
"""
tl_track = self.get(**criteria)
position = self._tl_tracks.index(tl_track)
del self._tl_tracks[position]
tl_tracks = self.filter(**criteria)
for tl_track in tl_tracks:
position = self._tl_tracks.index(tl_track)
del self._tl_tracks[position]
self.version += 1
return tl_tracks
def shuffle(self, start=None, end=None):
"""

View File

@ -103,12 +103,11 @@ def deleteid(context, tlid):
Deletes the song ``SONGID`` from the playlist
"""
try:
tlid = int(tlid)
if context.core.playback.current_tlid.get() == tlid:
context.core.playback.next()
return context.core.tracklist.remove(tlid=tlid).get()
except LookupError:
tlid = int(tlid)
if context.core.playback.current_tlid.get() == tlid:
context.core.playback.next()
tl_tracks = context.core.tracklist.remove(tlid=tlid).get()
if not tl_tracks:
raise MpdNoExistError('No such song', command='deleteid')
@ -163,8 +162,10 @@ def moveid(context, tlid, to):
"""
tlid = int(tlid)
to = int(to)
tl_track = context.core.tracklist.get(tlid=tlid).get()
position = context.core.tracklist.index(tl_track).get()
tl_tracks = context.core.tracklist.filter(tlid=tlid).get()
if not tl_tracks:
raise MpdNoExistError('No such song', command='moveid')
position = context.core.tracklist.index(tl_tracks[0]).get()
context.core.tracklist.move(position, position + 1, to)
@ -199,12 +200,11 @@ def playlistfind(context, tag, needle):
- does not add quotes around the tag.
"""
if tag == 'filename':
try:
tl_track = context.core.tracklist.get(uri=needle).get()
position = context.core.tracklist.index(tl_track).get()
return translator.track_to_mpd_format(tl_track, position=position)
except LookupError:
tl_tracks = context.core.tracklist.filter(uri=needle).get()
if not tl_tracks:
return None
position = context.core.tracklist.index(tl_tracks[0]).get()
return translator.track_to_mpd_format(tl_tracks[0], position=position)
raise MpdNotImplemented # TODO
@ -219,13 +219,12 @@ def playlistid(context, tlid=None):
and specifies a single song to display info for.
"""
if tlid is not None:
try:
tlid = int(tlid)
tl_track = context.core.tracklist.get(tlid=tlid).get()
position = context.core.tracklist.index(tl_track).get()
return translator.track_to_mpd_format(tl_track, position=position)
except LookupError:
tlid = int(tlid)
tl_tracks = context.core.tracklist.filter(tlid=tlid).get()
if not tl_tracks:
raise MpdNoExistError('No such song', command='playlistid')
position = context.core.tracklist.index(tl_tracks[0]).get()
return translator.track_to_mpd_format(tl_tracks[0], position=position)
else:
return translator.tracks_to_mpd_format(
context.core.tracklist.tl_tracks.get())
@ -385,8 +384,10 @@ def swapid(context, tlid1, tlid2):
"""
tlid1 = int(tlid1)
tlid2 = int(tlid2)
tl_track1 = context.core.tracklist.get(tlid=tlid1).get()
tl_track2 = context.core.tracklist.get(tlid=tlid2).get()
position1 = context.core.tracklist.index(tl_track1).get()
position2 = context.core.tracklist.index(tl_track2).get()
tl_tracks1 = context.core.tracklist.filter(tlid=tlid1).get()
tl_tracks2 = context.core.tracklist.filter(tlid=tlid2).get()
if not tl_tracks1 or not tl_tracks2:
raise MpdNoExistError('No such song', command='swapid')
position1 = context.core.tracklist.index(tl_tracks1[0]).get()
position2 = context.core.tracklist.index(tl_tracks2[0]).get()
swap(context, position1, position2)

View File

@ -151,11 +151,10 @@ def playid(context, tlid):
tlid = int(tlid)
if tlid == -1:
return _play_minus_one(context)
try:
tl_track = context.core.tracklist.get(tlid=tlid).get()
return context.core.playback.play(tl_track).get()
except LookupError:
tl_tracks = context.core.tracklist.filter(tlid=tlid).get()
if not tl_tracks:
raise MpdNoExistError('No such song', command='playid')
return context.core.playback.play(tl_tracks[0]).get()
@handle_request(r'^play (?P<songpos>-?\d+)$')

View File

@ -23,11 +23,10 @@ def listplaylist(context, name):
file: relative/path/to/file2.ogg
file: relative/path/to/file3.mp3
"""
try:
playlist = context.core.playlists.get(name=name).get()
return ['file: %s' % t.uri for t in playlist.tracks]
except LookupError:
playlists = context.core.playlists.filter(name=name).get()
if not playlists:
raise MpdNoExistError('No such playlist', command='listplaylist')
return ['file: %s' % t.uri for t in playlists[0].tracks]
@handle_request(r'^listplaylistinfo (?P<name>\S+)$')
@ -45,11 +44,10 @@ def listplaylistinfo(context, name):
Standard track listing, with fields: file, Time, Title, Date,
Album, Artist, Track
"""
try:
playlist = context.core.playlists.get(name=name).get()
return playlist_to_mpd_format(playlist)
except LookupError:
playlists = context.core.playlists.filter(name=name).get()
if not playlists:
raise MpdNoExistError('No such playlist', command='listplaylistinfo')
return playlist_to_mpd_format(playlists[0])
@handle_request(r'^listplaylists$')
@ -100,11 +98,10 @@ def load(context, name):
- ``load`` appends the given playlist to the current playlist.
"""
try:
playlist = context.core.playlists.get(name=name).get()
context.core.tracklist.append(playlist.tracks)
except LookupError:
playlists = context.core.playlists.filter(name=name).get()
if not playlists:
raise MpdNoExistError('No such playlist', command='load')
context.core.tracklist.append(playlists[0].tracks)
@handle_request(r'^playlistadd "(?P<name>[^"]+)" "(?P<uri>[^"]+)"$')

View File

@ -58,43 +58,35 @@ class PlaylistsControllerTest(object):
self.assertNotIn(playlist, self.core.playlists.playlists)
def test_get_without_criteria(self):
test = self.core.playlists.get
self.assertRaises(LookupError, test)
def test_filter_without_criteria(self):
self.assertEqual(
self.core.playlists.playlists, self.core.playlists.filter())
def test_get_with_wrong_cirteria(self):
test = lambda: self.core.playlists.get(name='foo')
self.assertRaises(LookupError, test)
def test_filter_with_wrong_criteria(self):
self.assertEqual([], self.core.playlists.filter(name='foo'))
def test_get_with_right_criteria(self):
playlist1 = self.core.playlists.create('test')
playlist2 = self.core.playlists.get(name='test')
self.assertEqual(playlist1, playlist2)
def test_filter_with_right_criteria(self):
playlist = self.core.playlists.create('test')
playlists = self.core.playlists.filter(name='test')
self.assertEqual([playlist], playlists)
def test_get_by_name_returns_unique_match(self):
def test_filter_by_name_returns_single_match(self):
playlist = Playlist(name='b')
self.backend.playlists.playlists = [
Playlist(name='a'), playlist]
self.assertEqual(playlist, self.core.playlists.get(name='b'))
self.backend.playlists.playlists = [Playlist(name='a'), playlist]
self.assertEqual([playlist], self.core.playlists.filter(name='b'))
def test_get_by_name_returns_first_of_multiple_matches(self):
def test_filter_by_name_returns_multiple_matches(self):
playlist = Playlist(name='b')
self.backend.playlists.playlists = [
playlist, Playlist(name='a'), Playlist(name='b')]
try:
self.core.playlists.get(name='b')
self.fail('Should raise LookupError if multiple matches')
except LookupError as e:
self.assertEqual('"name=b" match multiple playlists', e[0])
playlists = self.core.playlists.filter(name='b')
self.assertIn(playlist, playlists)
self.assertEqual(2, len(playlists))
def test_get_by_name_raises_keyerror_if_no_match(self):
def test_filter_by_name_returns_no_matches(self):
self.backend.playlists.playlists = [
Playlist(name='a'), Playlist(name='b')]
try:
self.core.playlists.get(name='c')
self.fail('Should raise LookupError if no match')
except LookupError as e:
self.assertEqual('"name=c" match no playlists', e[0])
self.assertEqual([], self.core.playlists.filter(name='c'))
def test_lookup_finds_playlist_by_uri(self):
original_playlist = self.core.playlists.create('test')

View File

@ -55,19 +55,56 @@ class TracklistControllerTest(object):
self.assertRaises(AssertionError, test)
@populate_playlist
def test_get_by_tlid(self):
def test_filter_by_tlid(self):
tl_track = self.controller.tl_tracks[1]
self.assertEqual(tl_track, self.controller.get(tlid=tl_track.tlid))
self.assertEqual(
[tl_track], self.controller.filter(tlid=tl_track.tlid))
@populate_playlist
def test_get_by_uri(self):
def test_filter_by_uri(self):
tl_track = self.controller.tl_tracks[1]
self.assertEqual(tl_track, self.controller.get(uri=tl_track.track.uri))
self.assertEqual(
[tl_track], self.controller.filter(uri=tl_track.track.uri))
@populate_playlist
def test_get_by_uri_raises_error_for_invalid_uri(self):
test = lambda: self.controller.get(uri='foobar')
self.assertRaises(LookupError, test)
def test_filter_by_uri_returns_nothing_for_invalid_uri(self):
self.assertEqual([], self.controller.filter(uri='foobar'))
def test_filter_by_uri_returns_single_match(self):
track = Track(uri='a')
self.controller.append([Track(uri='z'), track, Track(uri='y')])
self.assertEqual(track, self.controller.filter(uri='a')[0].track)
def test_filter_by_uri_returns_multiple_matches(self):
track = Track(uri='a')
self.controller.append([Track(uri='z'), track, track])
tl_tracks = self.controller.filter(uri='a')
self.assertEqual(track, tl_tracks[0].track)
self.assertEqual(track, tl_tracks[1].track)
def test_filter_by_uri_returns_nothing_if_no_match(self):
self.controller.playlist = Playlist(
tracks=[Track(uri='z'), Track(uri='y')])
self.assertEqual([], self.controller.filter(uri='a'))
def test_filter_by_multiple_criteria_returns_elements_matching_all(self):
track1 = Track(uri='a', name='x')
track2 = Track(uri='b', name='x')
track3 = Track(uri='b', name='y')
self.controller.append([track1, track2, track3])
self.assertEqual(
track1, self.controller.filter(uri='a', name='x')[0].track)
self.assertEqual(
track2, self.controller.filter(uri='b', name='x')[0].track)
self.assertEqual(
track3, self.controller.filter(uri='b', name='y')[0].track)
def test_filter_by_criteria_that_is_not_present_in_all_elements(self):
track1 = Track()
track2 = Track(uri='b')
track3 = Track()
self.controller.append([track1, track2, track3])
self.assertEqual(track2, self.controller.filter(uri='b')[0].track)
@populate_playlist
def test_clear(self):
@ -85,45 +122,6 @@ class TracklistControllerTest(object):
self.controller.clear()
self.assertEqual(self.playback.state, PlaybackState.STOPPED)
def test_get_by_uri_returns_unique_match(self):
track = Track(uri='a')
self.controller.append([Track(uri='z'), track, Track(uri='y')])
self.assertEqual(track, self.controller.get(uri='a').track)
def test_get_by_uri_raises_error_if_multiple_matches(self):
track = Track(uri='a')
self.controller.append([Track(uri='z'), track, track])
try:
self.controller.get(uri='a')
self.fail('Should raise LookupError if multiple matches')
except LookupError as e:
self.assertEqual('"uri=a" match multiple tracks', e[0])
def test_get_by_uri_raises_error_if_no_match(self):
self.controller.playlist = Playlist(
tracks=[Track(uri='z'), Track(uri='y')])
try:
self.controller.get(uri='a')
self.fail('Should raise LookupError if no match')
except LookupError as e:
self.assertEqual('"uri=a" match no tracks', e[0])
def test_get_by_multiple_criteria_returns_elements_matching_all(self):
track1 = Track(uri='a', name='x')
track2 = Track(uri='b', name='x')
track3 = Track(uri='b', name='y')
self.controller.append([track1, track2, track3])
self.assertEqual(track1, self.controller.get(uri='a', name='x').track)
self.assertEqual(track2, self.controller.get(uri='b', name='x').track)
self.assertEqual(track3, self.controller.get(uri='b', name='y').track)
def test_get_by_criteria_that_is_not_present_in_all_elements(self):
track1 = Track()
track2 = Track(uri='b')
track3 = Track()
self.controller.append([track1, track2, track3])
self.assertEqual(track2, self.controller.get(uri='b').track)
def test_append_appends_to_the_tracklist(self):
self.controller.append([Track(uri='a'), Track(uri='b')])
self.assertEqual(len(self.controller.tracks), 2)
@ -222,13 +220,11 @@ class TracklistControllerTest(object):
self.assertEqual(track2, self.controller.tracks[1])
@populate_playlist
def test_removing_track_that_does_not_exist(self):
test = lambda: self.controller.remove(uri='/nonexistant')
self.assertRaises(LookupError, test)
def test_removing_track_that_does_not_exist_does_nothing(self):
self.controller.remove(uri='/nonexistant')
def test_removing_from_empty_playlist(self):
test = lambda: self.controller.remove(uri='/nonexistant')
self.assertRaises(LookupError, test)
def test_removing_from_empty_playlist_does_nothing(self):
self.controller.remove(uri='/nonexistant')
@populate_playlist
def test_shuffle(self):

View File

@ -214,6 +214,11 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase):
self.assertEqual(tracks[5].name, 'f')
self.assertInResponse('OK')
def test_moveid_with_tlid_not_found_in_tracklist_should_ack(self):
self.sendRequest('moveid "9" "0"')
self.assertEqualResponse(
'ACK [50@0] {moveid} No such song')
def test_playlist_returns_same_as_playlistinfo(self):
playlist_response = self.sendRequest('playlist')
playlistinfo_response = self.sendRequest('playlistinfo')
@ -505,3 +510,15 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase):
self.assertEqual(tracks[4].name, 'b')
self.assertEqual(tracks[5].name, 'f')
self.assertInResponse('OK')
def test_swapid_with_first_id_unknown_should_ack(self):
self.core.tracklist.append([Track()])
self.sendRequest('swapid "0" "4"')
self.assertEqualResponse(
'ACK [50@0] {swapid} No such song')
def test_swapid_with_second_id_unknown_should_ack(self):
self.core.tracklist.append([Track()])
self.sendRequest('swapid "4" "0"')
self.assertEqualResponse(
'ACK [50@0] {swapid} No such song')