diff --git a/mopidy/frontends/mpd/dispatcher.py b/mopidy/frontends/mpd/dispatcher.py index e39c140bff..6590897db1 100644 --- a/mopidy/frontends/mpd/dispatcher.py +++ b/mopidy/frontends/mpd/dispatcher.py @@ -236,9 +236,6 @@ class MpdContext(object): #: The subsytems that we want to be notified about in idle mode. subscriptions = None - playlist_uri_from_name = None - playlist_name_from_uri = None - def __init__(self, dispatcher, session=None, config=None, core=None): self.dispatcher = dispatcher self.session = session @@ -246,14 +243,14 @@ def __init__(self, dispatcher, session=None, config=None, core=None): self.core = core self.events = set() self.subscriptions = set() - self.playlist_uri_from_name = {} - self.playlist_name_from_uri = {} + self._playlist_uri_from_name = {} + self._playlist_name_from_uri = {} self.refresh_playlists_mapping() def create_unique_name(self, playlist_name): name = playlist_name i = 2 - while name in self.playlist_uri_from_name: + while name in self._playlist_uri_from_name: name = '%s [%d]' % (playlist_name, i) i += 1 return name @@ -264,11 +261,30 @@ def refresh_playlists_mapping(self): MPD """ if self.core is not None: - self.playlist_uri_from_name.clear() - self.playlist_name_from_uri.clear() + self._playlist_uri_from_name.clear() + self._playlist_name_from_uri.clear() for playlist in self.core.playlists.playlists.get(): if not playlist.name: continue name = self.create_unique_name(playlist.name) - self.playlist_uri_from_name[name] = playlist.uri - self.playlist_name_from_uri[playlist.uri] = name + self._playlist_uri_from_name[name] = playlist.uri + self._playlist_name_from_uri[playlist.uri] = name + + def lookup_playlist_from_name(self, name): + """ + Helper function to retrieve a playlist from its unique MPD name. + """ + if not self._playlist_uri_from_name: + self.refresh_playlists_mapping() + if name not in self._playlist_uri_from_name: + return None + uri = self._playlist_uri_from_name[name] + return self.core.playlists.lookup(uri).get() + + def lookup_playlist_name_from_uri(self, uri): + """ + Helper function to retrieve the unique MPD playlist name from its uri. + """ + if uri not in self._playlist_name_from_uri: + self.refresh_playlists_mapping() + return self._playlist_name_from_uri[uri] diff --git a/mopidy/frontends/mpd/protocol/music_db.py b/mopidy/frontends/mpd/protocol/music_db.py index 11def30923..ff79c33a01 100644 --- a/mopidy/frontends/mpd/protocol/music_db.py +++ b/mopidy/frontends/mpd/protocol/music_db.py @@ -381,13 +381,8 @@ def searchaddpl(context, playlist_name, mpd_query): return results = context.core.library.search(**query).get() - if len(context.playlist_uri_from_name) == 0: - context.refresh_playlists_mapping() - - if playlist_name in context.playlist_uri_from_name: - uri = context.playlist_uri_from_name[playlist_name] - playlist = context.core.playlists.lookup(uri).get() - else: + playlist = context.lookup_playlist_from_name(playlist_name) + if not playlist: playlist = context.core.playlists.create(playlist_name).get() tracks = list(playlist.tracks) + _get_tracks(results) playlist = playlist.copy(tracks=tracks) diff --git a/mopidy/frontends/mpd/protocol/stored_playlists.py b/mopidy/frontends/mpd/protocol/stored_playlists.py index 0c9bf050d2..5b63fab4d0 100644 --- a/mopidy/frontends/mpd/protocol/stored_playlists.py +++ b/mopidy/frontends/mpd/protocol/stored_playlists.py @@ -23,10 +23,10 @@ def listplaylist(context, name): file: relative/path/to/file2.ogg file: relative/path/to/file3.mp3 """ - playlists = context.core.playlists.filter(name=name).get() - if not playlists: + playlist = context.lookup_playlist_from_name(name) + if not playlist: raise MpdNoExistError('No such playlist', command='listplaylist') - return ['file: %s' % t.uri for t in playlists[0].tracks] + return ['file: %s' % t.uri for t in playlist.tracks] @handle_request(r'^listplaylistinfo (?P\w+)$') @@ -44,10 +44,10 @@ def listplaylistinfo(context, name): Standard track listing, with fields: file, Time, Title, Date, Album, Artist, Track """ - playlists = context.core.playlists.filter(name=name).get() - if not playlists: + playlist = context.lookup_playlist_from_name(name) + if not playlist: raise MpdNoExistError('No such playlist', command='listplaylistinfo') - return playlist_to_mpd_format(playlists[0]) + return playlist_to_mpd_format(playlist) @handle_request(r'^listplaylists$') @@ -80,10 +80,7 @@ def listplaylists(context): for playlist in context.core.playlists.playlists.get(): if not playlist.name: continue - if playlist.uri not in context.playlist_name_from_uri: - # the maps are not synced, we refresh them - context.refresh_playlists_mapping() - name = context.playlist_name_from_uri[playlist.uri] + name = context.lookup_playlist_name_from_uri(playlist.uri) result.append(('playlist', name)) last_modified = ( playlist.last_modified or dt.datetime.utcnow()).isoformat() @@ -117,14 +114,14 @@ def load(context, name, start=None, end=None): - MPD 0.17.1 does not fail if the specified range is outside the playlist, in either or both ends. """ - playlists = context.core.playlists.filter(name=name).get() - if not playlists: + playlist = context.lookup_playlist_from_name(name) + if not playlist: raise MpdNoExistError('No such playlist', command='load') if start is not None: start = int(start) if end is not None: end = int(end) - context.core.tracklist.add(playlists[0].tracks[start:end]) + context.core.tracklist.add(playlist.tracks[start:end]) @handle_request(r'^playlistadd "(?P[^"]+)" "(?P[^"]+)"$') diff --git a/tests/frontends/mpd/protocol/stored_playlists_test.py b/tests/frontends/mpd/protocol/stored_playlists_test.py index d837b0facf..8199be2ba4 100644 --- a/tests/frontends/mpd/protocol/stored_playlists_test.py +++ b/tests/frontends/mpd/protocol/stored_playlists_test.py @@ -10,7 +10,8 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): def test_listplaylist(self): self.backend.playlists.playlists = [ - Playlist(name='name', tracks=[Track(uri='dummy:a')])] + Playlist(name='name', uri='dummy:name', + tracks=[Track(uri='dummy:a')])] self.sendRequest('listplaylist "name"') self.assertInResponse('file: dummy:a') @@ -18,7 +19,8 @@ def test_listplaylist(self): def test_listplaylist_without_quotes(self): self.backend.playlists.playlists = [ - Playlist(name='name', tracks=[Track(uri='dummy:a')])] + Playlist(name='name', uri='dummy:name', + tracks=[Track(uri='dummy:a')])] self.sendRequest('listplaylist name') self.assertInResponse('file: dummy:a') @@ -28,9 +30,19 @@ def test_listplaylist_fails_if_no_playlist_is_found(self): self.sendRequest('listplaylist "name"') self.assertEqualResponse('ACK [50@0] {listplaylist} No such playlist') + def test_listplaylist_duplicate(self): + playlist1 = Playlist(name='a', uri='dummy:a1', tracks=[Track(uri='b')]) + playlist2 = Playlist(name='a', uri='dummy:a2', tracks=[Track(uri='c')]) + self.backend.playlists.playlists = [playlist1, playlist2] + + self.sendRequest('listplaylist "a [2]"') + self.assertInResponse('file: c') + self.assertInResponse('OK') + def test_listplaylistinfo(self): self.backend.playlists.playlists = [ - Playlist(name='name', tracks=[Track(uri='dummy:a')])] + Playlist(name='name', uri='dummy:name', + tracks=[Track(uri='dummy:a')])] self.sendRequest('listplaylistinfo "name"') self.assertInResponse('file: dummy:a') @@ -40,7 +52,8 @@ def test_listplaylistinfo(self): def test_listplaylistinfo_without_quotes(self): self.backend.playlists.playlists = [ - Playlist(name='name', tracks=[Track(uri='dummy:a')])] + Playlist(name='name', uri='dummy:name', + tracks=[Track(uri='dummy:a')])] self.sendRequest('listplaylistinfo name') self.assertInResponse('file: dummy:a') @@ -53,10 +66,21 @@ def test_listplaylistinfo_fails_if_no_playlist_is_found(self): self.assertEqualResponse( 'ACK [50@0] {listplaylistinfo} No such playlist') + def test_listplaylistinfo_duplicate(self): + playlist1 = Playlist(name='a', uri='dummy:a1', tracks=[Track(uri='b')]) + playlist2 = Playlist(name='a', uri='dummy:a2', tracks=[Track(uri='c')]) + self.backend.playlists.playlists = [playlist1, playlist2] + + self.sendRequest('listplaylistinfo "a [2]"') + self.assertInResponse('file: c') + self.assertInResponse('Track: 0') + self.assertNotInResponse('Pos: 0') + self.assertInResponse('OK') + def test_listplaylists(self): last_modified = datetime.datetime(2001, 3, 17, 13, 41, 17, 12345) self.backend.playlists.playlists = [ - Playlist(name='a', last_modified=last_modified)] + Playlist(name='a', uri='dummy:a', last_modified=last_modified)] self.sendRequest('listplaylists') self.assertInResponse('playlist: a') @@ -77,7 +101,7 @@ def test_listplaylists_duplicate(self): def test_listplaylists_ignores_playlists_without_name(self): last_modified = datetime.datetime(2001, 3, 17, 13, 41, 17, 12345) self.backend.playlists.playlists = [ - Playlist(name='', last_modified=last_modified)] + Playlist(name='', uri='dummy:', last_modified=last_modified)] self.sendRequest('listplaylists') self.assertNotInResponse('playlist: ') @@ -87,7 +111,7 @@ def test_load_appends_to_tracklist(self): self.core.tracklist.add([Track(uri='a'), Track(uri='b')]) self.assertEqual(len(self.core.tracklist.tracks.get()), 2) self.backend.playlists.playlists = [ - Playlist(name='A-list', tracks=[ + Playlist(name='A-list', uri='dummy:A-list', tracks=[ Track(uri='c'), Track(uri='d'), Track(uri='e')])] self.sendRequest('load "A-list"') @@ -105,7 +129,7 @@ def test_load_with_range_loads_part_of_playlist(self): self.core.tracklist.add([Track(uri='a'), Track(uri='b')]) self.assertEqual(len(self.core.tracklist.tracks.get()), 2) self.backend.playlists.playlists = [ - Playlist(name='A-list', tracks=[ + Playlist(name='A-list', uri='dummy:A-list', tracks=[ Track(uri='c'), Track(uri='d'), Track(uri='e')])] self.sendRequest('load "A-list" "1:2"') @@ -121,7 +145,7 @@ def test_load_with_range_without_end_loads_rest_of_playlist(self): self.core.tracklist.add([Track(uri='a'), Track(uri='b')]) self.assertEqual(len(self.core.tracklist.tracks.get()), 2) self.backend.playlists.playlists = [ - Playlist(name='A-list', tracks=[ + Playlist(name='A-list', uri='dummy:A-list', tracks=[ Track(uri='c'), Track(uri='d'), Track(uri='e')])] self.sendRequest('load "A-list" "1:"')