diff --git a/docs/changes.rst b/docs/changes.rst index 4317e4ef..ffb7fbf6 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -128,6 +128,23 @@ backends: means that you now can select playlists to queue and play from the Ubuntu Sound Menu. +- :meth:`mopidy.core.LibraryController.find_exact` and + :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. + +- :meth:`mopidy.core.LibraryController.lookup` now returns a list of tracks. + This makes it possible to support lookup of artist or album URIs which then + can expand to a list of tracks. + **Bug fixes** - :issue:`218`: The MPD commands ``listplaylist`` and ``listplaylistinfo`` now @@ -143,6 +160,10 @@ backends: files (Apple lossless) because it didn't support multiple tag messages from GStreamer per track it scanned. +- :issue:`246`: The MPD command ``list album artist ""`` and similar + ``search``, ``find``, and ``list`` commands with empty filter values caused a + :exc:`LookupError`, but should have been ignored by the MPD server. + v0.8.1 (2012-10-30) =================== diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index a23c4c43..7de98075 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -89,7 +89,8 @@ class Audio(pykka.ThreadingActor): b'rate=(int)44100') source = element.get_property('source') source.set_property('caps', default_caps) - source.set_property('format', b'time') # Gstreamer does not like unicode + # GStreamer does not like unicode + source.set_property('format', b'time') self._appsrc = source @@ -219,8 +220,8 @@ class Audio(pykka.ThreadingActor): logger.debug( 'Triggering event: state_changed(old_state=%s, new_state=%s)', old_state, new_state) - AudioListener.send('state_changed', - old_state=old_state, new_state=new_state) + AudioListener.send( + 'state_changed', old_state=old_state, new_state=new_state) def _on_end_of_stream(self): logger.debug('Triggering reached_end_of_stream event') diff --git a/mopidy/backends/dummy.py b/mopidy/backends/dummy.py index d3239b34..62ac8e8f 100644 --- a/mopidy/backends/dummy.py +++ b/mopidy/backends/dummy.py @@ -39,18 +39,16 @@ class DummyLibraryProvider(base.BaseLibraryProvider): self.dummy_library = [] def find_exact(self, **query): - return Playlist() + return [] def lookup(self, uri): - matches = filter(lambda t: uri == t.uri, self.dummy_library) - if matches: - return matches[0] + return filter(lambda t: uri == t.uri, self.dummy_library) def refresh(self, uri=None): pass def search(self, **query): - return Playlist() + return [] class DummyPlaybackProvider(base.BasePlaybackProvider): diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index 3454ca76..e0e6f423 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -4,7 +4,7 @@ import logging from mopidy import settings from mopidy.backends import base -from mopidy.models import Playlist, Album +from mopidy.models import Album from .translator import parse_mpd_tag_cache @@ -30,10 +30,10 @@ class LocalLibraryProvider(base.BaseLibraryProvider): def lookup(self, uri): try: - return self._uri_mapping[uri] + return [self._uri_mapping[uri]] except KeyError: logger.debug('Failed to lookup %r', uri) - return None + return [] def find_exact(self, **query): self._validate_query(query) @@ -67,7 +67,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return Playlist(tracks=result_tracks) + return result_tracks def search(self, **query): self._validate_query(query) @@ -101,7 +101,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return Playlist(tracks=result_tracks) + return result_tracks def _validate_query(self, query): for (_, values) in query.iteritems(): diff --git a/mopidy/backends/local/playlists.py b/mopidy/backends/local/playlists.py index ea45bcbb..666532c5 100644 --- a/mopidy/backends/local/playlists.py +++ b/mopidy/backends/local/playlists.py @@ -55,7 +55,7 @@ class LocalPlaylistsProvider(base.BasePlaylistsProvider): try: # TODO We must use core.library.lookup() to support tracks # from other backends - tracks.append(self.backend.library.lookup(track_uri)) + tracks += self.backend.library.lookup(track_uri) except LookupError as ex: logger.error('Playlist item could not be added: %s', ex) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 67c390fc..df04058b 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -6,7 +6,7 @@ import Queue from spotify import Link, SpotifyError from mopidy.backends import base -from mopidy.models import Track, Playlist +from mopidy.models import Track from . import translator @@ -57,10 +57,10 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): def lookup(self, uri): try: - return SpotifyTrack(uri) + return [SpotifyTrack(uri)] except SpotifyError as e: logger.debug('Failed to lookup "%s": %s', uri, e) - return None + return [] def refresh(self, uri=None): pass # TODO @@ -72,7 +72,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): tracks = [] for playlist in self.backend.playlists.playlists: tracks += playlist.tracks - return Playlist(tracks=tracks) + return tracks spotify_query = [] for (field, values) in query.iteritems(): if field == 'uri': @@ -81,7 +81,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): track = self.lookup(value) if track: tracks.append(track) - return Playlist(tracks=tracks) + return tracks elif field == 'track': field = 'title' elif field == 'date': @@ -103,4 +103,4 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): try: return queue.get(timeout=3) # XXX What is an reasonable timeout? except Queue.Empty: - return Playlist(tracks=[]) + return [] diff --git a/mopidy/backends/spotify/session_manager.py b/mopidy/backends/spotify/session_manager.py index b46fd659..821bd27c 100644 --- a/mopidy/backends/spotify/session_manager.py +++ b/mopidy/backends/spotify/session_manager.py @@ -12,7 +12,6 @@ from spotify.manager import SpotifySessionManager as PyspotifySessionManager from mopidy import settings from mopidy.backends.listener import BackendListener -from mopidy.models import Playlist from mopidy.utils import process, versioning from . import translator @@ -164,9 +163,9 @@ class SpotifySessionManager(process.BaseThread, PyspotifySessionManager): # TODO Include results from results.albums(), etc. too # TODO Consider launching a second search if results.total_tracks() # is larger than len(results.tracks()) - playlist = Playlist(tracks=[ - translator.to_mopidy_track(t) for t in results.tracks()]) - queue.put(playlist) + tracks = [ + translator.to_mopidy_track(t) for t in results.tracks()] + queue.put(tracks) self.connected.wait() self.session.search( query, callback, track_count=100, album_count=0, artist_count=0) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 58263fd1..c1a89222 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -5,8 +5,6 @@ import urlparse import pykka -from mopidy.models import Playlist - class LibraryController(object): pykka_traversable = True @@ -34,27 +32,29 @@ class LibraryController(object): :param query: one or more queries to search for :type query: dict - :rtype: :class:`mopidy.models.Playlist` + :rtype: list of :class:`mopidy.models.Track` """ - futures = [b.library.find_exact(**query) - for b in self.backends.with_library] + futures = [ + b.library.find_exact(**query) for b in self.backends.with_library] results = pykka.get_all(futures) - return Playlist(tracks=[ - track for playlist in results for track in playlist.tracks]) + return list(itertools.chain(*results)) def lookup(self, uri): """ - Lookup track with given URI. Returns :class:`None` if not found. + Lookup the given URI. + + If the URI expands to multiple tracks, the returned list will contain + them all. :param uri: track URI :type uri: string - :rtype: :class:`mopidy.models.Track` or :class:`None` + :rtype: list of :class:`mopidy.models.Track` """ backend = self._get_backend(uri) if backend: return backend.library.lookup(uri).get() else: - return None + return [] def refresh(self, uri=None): """ @@ -68,8 +68,8 @@ class LibraryController(object): if backend: backend.library.refresh(uri).get() else: - futures = [b.library.refresh(uri) - for b in self.backends.with_library] + futures = [ + b.library.refresh(uri) for b in self.backends.with_library] pykka.get_all(futures) def search(self, **query): @@ -87,11 +87,9 @@ class LibraryController(object): :param query: one or more queries to search for :type query: dict - :rtype: :class:`mopidy.models.Playlist` + :rtype: list of :class:`mopidy.models.Track` """ - futures = [b.library.search(**query) - for b in self.backends.with_library] + futures = [ + b.library.search(**query) for b in self.backends.with_library] results = pykka.get_all(futures) - track_lists = [playlist.tracks for playlist in results] - tracks = list(itertools.chain(*track_lists)) - return Playlist(tracks=tracks) + return list(itertools.chain(*results)) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 273eb68d..94b4af9c 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -38,9 +38,7 @@ class PlaybackController(object): #: Tracks are not removed from the playlist. consume = option_wrapper('_consume', False) - #: The currently playing or selected track. - #: - #: A two-tuple of (TLID integer, :class:`mopidy.models.Track`) or + #: The currently playing or selected :class:`mopidy.models.TlTrack`, or #: :class:`None`. current_tl_track = None @@ -139,7 +137,7 @@ class PlaybackController(object): """ The track that will be played at the end of the current track. - Read-only. A two-tuple of (TLID integer, :class:`mopidy.models.Track`). + Read-only. A :class:`mopidy.models.TlTrack`. Not necessarily the same track as :attr:`tl_track_at_next`. """ @@ -190,7 +188,7 @@ class PlaybackController(object): """ The track that will be played if calling :meth:`next()`. - Read-only. A two-tuple of (TLID integer, :class:`mopidy.models.Track`). + Read-only. A :class:`mopidy.models.TlTrack`. For normal playback this is the next track in the playlist. If repeat is enabled the next track can loop around the playlist. When random is @@ -238,7 +236,7 @@ class PlaybackController(object): """ The track that will be played if calling :meth:`previous()`. - A two-tuple of (TLID integer, :class:`mopidy.models.Track`). + A :class:`mopidy.models.TlTrack`. For normal playback this is the previous track in the playlist. If random and/or consume is enabled it should return the current track @@ -310,12 +308,10 @@ class PlaybackController(object): Change to the given track, keeping the current playback state. :param tl_track: track to change to - :type tl_track: two-tuple (TLID integer, :class:`mopidy.models.Track`) - or :class:`None` + :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None` :param on_error_step: direction to step at play error, 1 for next track (default), -1 for previous track :type on_error_step: int, -1 or 1 - """ old_state = self.state self.stop() @@ -383,8 +379,7 @@ class PlaybackController(object): currently active track. :param tl_track: track to play - :type tl_track: two-tuple (TLID integer, :class:`mopidy.models.Track`) - or :class:`None` + :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None` :param on_error_step: direction to step at play error, 1 for next track (default), -1 for previous track :type on_error_step: int, -1 or 1 diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 25ae2bdf..dcdc665f 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -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): """ diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 4a628d81..e00a42f9 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -23,7 +23,7 @@ class TracklistController(object): @property def tl_tracks(self): """ - List of two-tuples of (TLID integer, :class:`mopidy.models.Track`). + List of :class:`mopidy.models.TlTrack`. Read-only. """ @@ -72,8 +72,7 @@ class TracklistController(object): :type at_position: int or :class:`None` :param increase_version: if the tracklist version should be increased :type increase_version: :class:`True` or :class:`False` - :rtype: two-tuple of (TLID integer, :class:`mopidy.models.Track`) that - was added to the tracklist + :rtype: :class:`mopidy.models.TlTrack` that was added to the tracklist """ assert at_position <= len(self._tl_tracks), \ 'at_position can not be greater than tracklist length' @@ -117,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: two-tuple (TLID integer, :class:`mopidy.models.Track`) + :rtype: list of :class:`mopidy.models.TlTrack` """ matches = self._tl_tracks for (key, value) in criteria.iteritems(): @@ -141,24 +138,16 @@ 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): """ - Get index of the given (TLID integer, :class:`mopidy.models.Track`) - two-tuple in the tracklist. + Get index of the given :class:`mopidy.models.TlTrack` in the tracklist. Raises :exc:`ValueError` if not found. :param tl_track: track to find the index of - :type tl_track: two-tuple (TLID integer, :class:`mopidy.models.Track`) + :type tl_track: :class:`mopidy.models.TlTrack` :rtype: int """ return self._tl_tracks.index(tl_track) @@ -199,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): """ @@ -255,7 +247,7 @@ class TracklistController(object): :type start: int :param end: position after last track to include in slice :type end: int - :rtype: two-tuple of (TLID integer, :class:`mopidy.models.Track`) + :rtype: :class:`mopidy.models.TlTrack` """ return self._tl_tracks[start:end] diff --git a/mopidy/frontends/mpd/protocol/current_playlist.py b/mopidy/frontends/mpd/protocol/current_playlist.py index 1a3f4f7b..da950078 100644 --- a/mopidy/frontends/mpd/protocol/current_playlist.py +++ b/mopidy/frontends/mpd/protocol/current_playlist.py @@ -22,9 +22,9 @@ def add(context, uri): """ if not uri: return - track = context.core.library.lookup(uri).get() - if track: - context.core.tracklist.add(track) + tracks = context.core.library.lookup(uri).get() + if tracks: + context.core.tracklist.append(tracks) return raise MpdNoExistError('directory or file not found', command='add') @@ -52,13 +52,19 @@ def addid(context, uri, songpos=None): raise MpdNoExistError('No such song', command='addid') if songpos is not None: songpos = int(songpos) - track = context.core.library.lookup(uri).get() - if track is None: + tracks = context.core.library.lookup(uri).get() + if not tracks: raise MpdNoExistError('No such song', command='addid') if songpos and songpos > context.core.tracklist.length.get(): raise MpdArgError('Bad song index', command='addid') - tl_track = context.core.tracklist.add(track, at_position=songpos).get() - return ('Id', tl_track.tlid) + first_tl_track = None + for track in tracks: + tl_track = context.core.tracklist.add(track, at_position=songpos).get() + if songpos is not None: + songpos += 1 + if first_tl_track is None: + first_tl_track = tl_track + return ('Id', first_tl_track.tlid) @handle_request(r'^delete "(?P\d+):(?P\d+)*"$') @@ -103,12 +109,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 +168,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 +206,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 +225,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 +390,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) diff --git a/mopidy/frontends/mpd/protocol/music_db.py b/mopidy/frontends/mpd/protocol/music_db.py index bea57198..4d6433f1 100644 --- a/mopidy/frontends/mpd/protocol/music_db.py +++ b/mopidy/frontends/mpd/protocol/music_db.py @@ -5,7 +5,7 @@ import shlex from mopidy.frontends.mpd.exceptions import MpdArgError, MpdNotImplemented from mopidy.frontends.mpd.protocol import handle_request, stored_playlists -from mopidy.frontends.mpd.translator import playlist_to_mpd_format +from mopidy.frontends.mpd.translator import tracks_to_mpd_format def _build_query(mpd_query): @@ -28,6 +28,8 @@ def _build_query(mpd_query): field = 'uri' field = str(field) # Needed for kwargs keys on OS X and Windows what = m.groupdict()['what'] + if not what: + raise ValueError if field in query: query[field].append(what) else: @@ -76,8 +78,11 @@ def find(context, mpd_query): - also uses the search type "date". - uses "file" instead of "filename". """ - query = _build_query(mpd_query) - return playlist_to_mpd_format( + try: + query = _build_query(mpd_query) + except ValueError: + return + return tracks_to_mpd_format( context.core.library.find_exact(**query).get()) @@ -185,7 +190,10 @@ def list_(context, field, mpd_query=None): - capitalizes the field argument. """ field = field.lower() - query = _list_build_query(field, mpd_query) + try: + query = _list_build_query(field, mpd_query) + except ValueError: + return if field == 'artist': return _list_artist(context, query) elif field == 'album': @@ -211,6 +219,8 @@ def _list_build_query(field, mpd_query): tokens = [t.decode('utf-8') for t in tokens] if len(tokens) == 1: if field == 'album': + if not tokens[0]: + raise ValueError return {'artist': [tokens[0]]} else: raise MpdArgError( @@ -224,6 +234,8 @@ def _list_build_query(field, mpd_query): tokens = tokens[2:] if key not in ('artist', 'album', 'date', 'genre'): raise MpdArgError('not able to parse args', command='list') + if not value: + raise ValueError if key in query: query[key].append(value) else: @@ -235,8 +247,8 @@ def _list_build_query(field, mpd_query): def _list_artist(context, query): artists = set() - playlist = context.core.library.find_exact(**query).get() - for track in playlist.tracks: + tracks = context.core.library.find_exact(**query).get() + for track in tracks: for artist in track.artists: artists.add(('Artist', artist.name)) return artists @@ -244,8 +256,8 @@ def _list_artist(context, query): def _list_album(context, query): albums = set() - playlist = context.core.library.find_exact(**query).get() - for track in playlist.tracks: + tracks = context.core.library.find_exact(**query).get() + for track in tracks: if track.album is not None: albums.add(('Album', track.album.name)) return albums @@ -253,8 +265,8 @@ def _list_album(context, query): def _list_date(context, query): dates = set() - playlist = context.core.library.find_exact(**query).get() - for track in playlist.tracks: + tracks = context.core.library.find_exact(**query).get() + for track in tracks: if track.date is not None: dates.add(('Date', track.date)) return dates @@ -351,8 +363,11 @@ def search(context, mpd_query): - also uses the search type "date". - uses "file" instead of "filename". """ - query = _build_query(mpd_query) - return playlist_to_mpd_format( + try: + query = _build_query(mpd_query) + except ValueError: + return + return tracks_to_mpd_format( context.core.library.search(**query).get()) diff --git a/mopidy/frontends/mpd/protocol/playback.py b/mopidy/frontends/mpd/protocol/playback.py index 74ecfb1c..d166f982 100644 --- a/mopidy/frontends/mpd/protocol/playback.py +++ b/mopidy/frontends/mpd/protocol/playback.py @@ -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-?\d+)$') diff --git a/mopidy/frontends/mpd/protocol/stored_playlists.py b/mopidy/frontends/mpd/protocol/stored_playlists.py index b8ac8c4c..d5d6b2a6 100644 --- a/mopidy/frontends/mpd/protocol/stored_playlists.py +++ b/mopidy/frontends/mpd/protocol/stored_playlists.py @@ -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\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[^"]+)" "(?P[^"]+)"$') diff --git a/mopidy/frontends/mpris/objects.py b/mopidy/frontends/mpris/objects.py index a66abdb5..51b0d7e8 100644 --- a/mopidy/frontends/mpris/objects.py +++ b/mopidy/frontends/mpris/objects.py @@ -279,13 +279,10 @@ class MprisObject(dbus.service.Object): return # NOTE Check if URI has MIME type known to the backend, if MIME support # is added to the backend. - uri_schemes = self.core.uri_schemes.get() - if not any([uri.startswith(uri_scheme) for uri_scheme in uri_schemes]): - return - track = self.core.library.lookup(uri).get() - if track is not None: - tl_track = self.core.tracklist.add(track).get() - self.core.playback.play(tl_track) + tracks = self.core.library.lookup(uri).get() + if tracks: + tl_tracks = self.core.tracklist.append(tracks).get() + self.core.playback.play(tl_tracks[0]) else: logger.debug('Track with URI "%s" not found in library.', uri) diff --git a/mopidy/models.py b/mopidy/models.py index 4861ef0d..a4ed1b4f 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -14,7 +14,7 @@ class ImmutableObject(object): def __init__(self, *args, **kwargs): for key, value in kwargs.items(): - if not hasattr(self, key): + if not hasattr(self, key) or callable(getattr(self, key)): raise TypeError( '__init__() got an unexpected keyword argument "%s"' % key) @@ -80,7 +80,7 @@ class ImmutableObject(object): def serialize(self): data = {} - data['__type__'] = self.__class__.__name__ + data['__model__'] = self.__class__.__name__ for key in self.__dict__.keys(): public_key = key.lstrip('_') value = self.__dict__[key] @@ -101,7 +101,7 @@ class ModelJSONEncoder(json.JSONEncoder): >>> import json >>> json.dumps({'a_track': Track(name='name')}, cls=ModelJSONEncoder) - '{"a_track": {"__type__": "Track", "name": "name"}}' + '{"a_track": {"__model__": "Track", "name": "name"}}' """ def default(self, obj): @@ -118,23 +118,16 @@ def model_json_decoder(dct): >>> import json >>> json.loads( - ... '{"a_track": {"__type__": "Track", "name": "name"}}', + ... '{"a_track": {"__model__": "Track", "name": "name"}}', ... object_hook=model_json_decoder) {u'a_track': Track(artists=[], name=u'name')} """ - if '__type__' in dct: - obj_type = dct.pop('__type__') - if obj_type == 'Album': - return Album(**dct) - if obj_type == 'Artist': - return Artist(**dct) - if obj_type == 'Playlist': - return Playlist(**dct) - if obj_type == 'TlTrack': - return TlTrack(**dct) - if obj_type == 'Track': - return Track(**dct) + if '__model__' in dct: + model_name = dct.pop('__model__') + cls = globals().get(model_name, None) + if issubclass(cls, ImmutableObject): + return cls(**dct) return dct diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 3ddfe2ee..604350d1 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -317,7 +317,7 @@ class LineProtocol(pykka.ThreadingActor): super(LineProtocol, self).__init__() self.connection = connection self.prevent_timeout = False - self.recv_buffer = '' + self.recv_buffer = b'' if self.delimiter: self.delimiter = re.compile(self.delimiter) diff --git a/mopidy/utils/process.py b/mopidy/utils/process.py index 27e312de..5edf287e 100644 --- a/mopidy/utils/process.py +++ b/mopidy/utils/process.py @@ -16,7 +16,8 @@ from mopidy import exceptions logger = logging.getLogger('mopidy.utils.process') -SIGNALS = dict((k, v) for v, k in signal.__dict__.iteritems() +SIGNALS = dict( + (k, v) for v, k in signal.__dict__.iteritems() if v.startswith('SIG') and not v.startswith('SIG_')) @@ -98,7 +99,8 @@ class DebugThread(threading.Thread): for ident, frame in sys._current_frames().items(): if self.ident != ident: stack = ''.join(traceback.format_stack(frame)) - logger.debug('Current state of %s (%s):\n%s', + logger.debug( + 'Current state of %s (%s):\n%s', threads[ident], ident, stack) del frame diff --git a/tests/backends/base/library.py b/tests/backends/base/library.py index 0b32186f..4e9232e5 100644 --- a/tests/backends/base/library.py +++ b/tests/backends/base/library.py @@ -3,7 +3,7 @@ from __future__ import unicode_literals import pykka from mopidy import core -from mopidy.models import Playlist, Track, Album, Artist +from mopidy.models import Track, Album, Artist from tests import unittest, path_to_data_dir @@ -43,52 +43,52 @@ class LibraryControllerTest(object): pass def test_lookup(self): - track = self.library.lookup(self.tracks[0].uri) - self.assertEqual(track, self.tracks[0]) + tracks = self.library.lookup(self.tracks[0].uri) + self.assertEqual(tracks, self.tracks[0:1]) def test_lookup_unknown_track(self): - track = self.library.lookup('fake uri') - self.assertEquals(track, None) + tracks = self.library.lookup('fake uri') + self.assertEqual(tracks, []) def test_find_exact_no_hits(self): result = self.library.find_exact(track=['unknown track']) - self.assertEqual(result, Playlist()) + self.assertEqual(result, []) result = self.library.find_exact(artist=['unknown artist']) - self.assertEqual(result, Playlist()) + self.assertEqual(result, []) result = self.library.find_exact(album=['unknown artist']) - self.assertEqual(result, Playlist()) + self.assertEqual(result, []) def test_find_exact_artist(self): result = self.library.find_exact(artist=['artist1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.find_exact(artist=['artist2']) - self.assertEqual(result, Playlist(tracks=self.tracks[1:2])) + self.assertEqual(result, self.tracks[1:2]) def test_find_exact_track(self): result = self.library.find_exact(track=['track1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.find_exact(track=['track2']) - self.assertEqual(result, Playlist(tracks=self.tracks[1:2])) + self.assertEqual(result, self.tracks[1:2]) def test_find_exact_album(self): result = self.library.find_exact(album=['album1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.find_exact(album=['album2']) - self.assertEqual(result, Playlist(tracks=self.tracks[1:2])) + self.assertEqual(result, self.tracks[1:2]) def test_find_exact_uri(self): track_1_uri = 'file://' + path_to_data_dir('uri1') result = self.library.find_exact(uri=track_1_uri) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) track_2_uri = 'file://' + path_to_data_dir('uri2') result = self.library.find_exact(uri=track_2_uri) - self.assertEqual(result, Playlist(tracks=self.tracks[1:2])) + self.assertEqual(result, self.tracks[1:2]) def test_find_exact_wrong_type(self): test = lambda: self.library.find_exact(wrong=['test']) @@ -106,57 +106,57 @@ class LibraryControllerTest(object): def test_search_no_hits(self): result = self.library.search(track=['unknown track']) - self.assertEqual(result, Playlist()) + self.assertEqual(result, []) result = self.library.search(artist=['unknown artist']) - self.assertEqual(result, Playlist()) + self.assertEqual(result, []) result = self.library.search(album=['unknown artist']) - self.assertEqual(result, Playlist()) + self.assertEqual(result, []) result = self.library.search(uri=['unknown']) - self.assertEqual(result, Playlist()) + self.assertEqual(result, []) result = self.library.search(any=['unknown']) - self.assertEqual(result, Playlist()) + self.assertEqual(result, []) def test_search_artist(self): result = self.library.search(artist=['Tist1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.search(artist=['Tist2']) - self.assertEqual(result, Playlist(tracks=self.tracks[1:2])) + self.assertEqual(result, self.tracks[1:2]) def test_search_track(self): result = self.library.search(track=['Rack1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.search(track=['Rack2']) - self.assertEqual(result, Playlist(tracks=self.tracks[1:2])) + self.assertEqual(result, self.tracks[1:2]) def test_search_album(self): result = self.library.search(album=['Bum1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.search(album=['Bum2']) - self.assertEqual(result, Playlist(tracks=self.tracks[1:2])) + self.assertEqual(result, self.tracks[1:2]) def test_search_uri(self): result = self.library.search(uri=['RI1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.search(uri=['RI2']) - self.assertEqual(result, Playlist(tracks=self.tracks[1:2])) + self.assertEqual(result, self.tracks[1:2]) def test_search_any(self): result = self.library.search(any=['Tist1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.search(any=['Rack1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.search(any=['Bum1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) result = self.library.search(any=['RI1']) - self.assertEqual(result, Playlist(tracks=self.tracks[:1])) + self.assertEqual(result, self.tracks[:1]) def test_search_wrong_type(self): test = lambda: self.library.search(wrong=['test']) diff --git a/tests/backends/base/playlists.py b/tests/backends/base/playlists.py index 473caf8c..c162e500 100644 --- a/tests/backends/base/playlists.py +++ b/tests/backends/base/playlists.py @@ -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') diff --git a/tests/backends/base/tracklist.py b/tests/backends/base/tracklist.py index 65328f60..a5fbbcb5 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -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): diff --git a/tests/core/library_test.py b/tests/core/library_test.py index 7886b85c..1bd481de 100644 --- a/tests/core/library_test.py +++ b/tests/core/library_test.py @@ -4,7 +4,7 @@ import mock from mopidy.backends import base from mopidy.core import Core -from mopidy.models import Playlist, Track +from mopidy.models import Track from tests import unittest @@ -41,10 +41,10 @@ class CoreLibraryTest(unittest.TestCase): self.assertFalse(self.library1.lookup.called) self.library2.lookup.assert_called_once_with('dummy2:a') - def test_lookup_fails_for_dummy3_track(self): + def test_lookup_returns_nothing_for_dummy3_track(self): result = self.core.library.lookup('dummy3:a') - self.assertIsNone(result) + self.assertEqual(result, []) self.assertFalse(self.library1.lookup.called) self.assertFalse(self.library2.lookup.called) @@ -75,29 +75,29 @@ class CoreLibraryTest(unittest.TestCase): def test_find_exact_combines_results_from_all_backends(self): track1 = Track(uri='dummy1:a') track2 = Track(uri='dummy2:a') - self.library1.find_exact().get.return_value = Playlist(tracks=[track1]) + self.library1.find_exact().get.return_value = [track1] self.library1.find_exact.reset_mock() - self.library2.find_exact().get.return_value = Playlist(tracks=[track2]) + self.library2.find_exact().get.return_value = [track2] self.library2.find_exact.reset_mock() result = self.core.library.find_exact(any=['a']) - self.assertIn(track1, result.tracks) - self.assertIn(track2, result.tracks) + self.assertIn(track1, result) + self.assertIn(track2, result) self.library1.find_exact.assert_called_once_with(any=['a']) self.library2.find_exact.assert_called_once_with(any=['a']) def test_search_combines_results_from_all_backends(self): track1 = Track(uri='dummy1:a') track2 = Track(uri='dummy2:a') - self.library1.search().get.return_value = Playlist(tracks=[track1]) + self.library1.search().get.return_value = [track1] self.library1.search.reset_mock() - self.library2.search().get.return_value = Playlist(tracks=[track2]) + self.library2.search().get.return_value = [track2] self.library2.search.reset_mock() result = self.core.library.search(any=['a']) - self.assertIn(track1, result.tracks) - self.assertIn(track2, result.tracks) + self.assertIn(track1, result) + self.assertIn(track2, result) self.library1.search.assert_called_once_with(any=['a']) self.library2.search.assert_called_once_with(any=['a']) diff --git a/tests/core/playback_test.py b/tests/core/playback_test.py index 8e83f971..bb3d359f 100644 --- a/tests/core/playback_test.py +++ b/tests/core/playback_test.py @@ -58,8 +58,8 @@ class CorePlaybackTest(unittest.TestCase): self.playback1.play.assert_called_once_with(self.tracks[3]) self.assertFalse(self.playback2.play.called) - self.assertEqual(self.core.playback.current_tl_track, - self.tl_tracks[3]) + self.assertEqual( + self.core.playback.current_tl_track, self.tl_tracks[3]) def test_pause_selects_dummy1_backend(self): self.core.playback.play(self.tl_tracks[0]) diff --git a/tests/frontends/mpd/protocol/current_playlist_test.py b/tests/frontends/mpd/protocol/current_playlist_test.py index f5f15f81..dd1ba57e 100644 --- a/tests/frontends/mpd/protocol/current_playlist_test.py +++ b/tests/frontends/mpd/protocol/current_playlist_test.py @@ -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') diff --git a/tests/frontends/mpd/protocol/playback_test.py b/tests/frontends/mpd/protocol/playback_test.py index 51468390..f81be241 100644 --- a/tests/frontends/mpd/protocol/playback_test.py +++ b/tests/frontends/mpd/protocol/playback_test.py @@ -236,8 +236,8 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.sendRequest('play "-1"') self.assertEqual(PLAYING, self.core.playback.state.get()) - self.assertEqual('dummy:a', - self.core.playback.current_track.get().uri) + self.assertEqual( + 'dummy:a', self.core.playback.current_track.get().uri) self.assertInResponse('OK') def test_play_minus_one_plays_current_track_if_current_track_is_set(self): @@ -253,8 +253,8 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.sendRequest('play "-1"') self.assertEqual(PLAYING, self.core.playback.state.get()) - self.assertEqual('dummy:b', - self.core.playback.current_track.get().uri) + self.assertEqual( + 'dummy:b', self.core.playback.current_track.get().uri) self.assertInResponse('OK') def test_play_minus_one_on_empty_playlist_does_not_ack(self): @@ -318,8 +318,8 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.sendRequest('playid "-1"') self.assertEqual(PLAYING, self.core.playback.state.get()) - self.assertEqual('dummy:a', - self.core.playback.current_track.get().uri) + self.assertEqual( + 'dummy:a', self.core.playback.current_track.get().uri) self.assertInResponse('OK') def test_playid_minus_1_plays_current_track_if_current_track_is_set(self): @@ -335,8 +335,8 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.sendRequest('playid "-1"') self.assertEqual(PLAYING, self.core.playback.state.get()) - self.assertEqual('dummy:b', - self.core.playback.current_track.get().uri) + self.assertEqual( + 'dummy:b', self.core.playback.current_track.get().uri) self.assertInResponse('OK') def test_playid_minus_one_on_empty_playlist_does_not_ack(self): diff --git a/tests/frontends/mpd/protocol/regression_test.py b/tests/frontends/mpd/protocol/regression_test.py index 68230c6a..6b8832e4 100644 --- a/tests/frontends/mpd/protocol/regression_test.py +++ b/tests/frontends/mpd/protocol/regression_test.py @@ -29,22 +29,22 @@ class IssueGH17RegressionTest(protocol.BaseTestCase): random.seed(1) # Playlist order: abcfde self.sendRequest('play') - self.assertEquals('dummy:a', - self.core.playback.current_track.get().uri) + self.assertEquals( + 'dummy:a', self.core.playback.current_track.get().uri) self.sendRequest('random "1"') self.sendRequest('next') - self.assertEquals('dummy:b', - self.core.playback.current_track.get().uri) + self.assertEquals( + 'dummy:b', self.core.playback.current_track.get().uri) self.sendRequest('next') # Should now be at track 'c', but playback fails and it skips ahead - self.assertEquals('dummy:f', - self.core.playback.current_track.get().uri) + self.assertEquals( + 'dummy:f', self.core.playback.current_track.get().uri) self.sendRequest('next') - self.assertEquals('dummy:d', - self.core.playback.current_track.get().uri) + self.assertEquals( + 'dummy:d', self.core.playback.current_track.get().uri) self.sendRequest('next') - self.assertEquals('dummy:e', - self.core.playback.current_track.get().uri) + self.assertEquals( + 'dummy:e', self.core.playback.current_track.get().uri) class IssueGH18RegressionTest(protocol.BaseTestCase): diff --git a/tests/frontends/mpris/player_interface_test.py b/tests/frontends/mpris/player_interface_test.py index 35fb0161..39b77093 100644 --- a/tests/frontends/mpris/player_interface_test.py +++ b/tests/frontends/mpris/player_interface_test.py @@ -812,23 +812,20 @@ class PlayerInterfaceTest(unittest.TestCase): def test_open_uri_ignores_uris_with_unknown_uri_scheme(self): self.assertListEqual(self.core.uri_schemes.get(), ['dummy']) self.mpris.get_CanPlay = lambda *_: True - self.backend.library.dummy_library = [ - Track(uri='notdummy:/test/uri')] + self.backend.library.dummy_library = [Track(uri='notdummy:/test/uri')] self.mpris.OpenUri('notdummy:/test/uri') self.assertEqual(len(self.core.tracklist.tracks.get()), 0) def test_open_uri_adds_uri_to_tracklist(self): self.mpris.get_CanPlay = lambda *_: True - self.backend.library.dummy_library = [ - Track(uri='dummy:/test/uri')] + self.backend.library.dummy_library = [Track(uri='dummy:/test/uri')] self.mpris.OpenUri('dummy:/test/uri') self.assertEqual( self.core.tracklist.tracks.get()[0].uri, 'dummy:/test/uri') def test_open_uri_starts_playback_of_new_track_if_stopped(self): self.mpris.get_CanPlay = lambda *_: True - self.backend.library.dummy_library = [ - Track(uri='dummy:/test/uri')] + self.backend.library.dummy_library = [Track(uri='dummy:/test/uri')] self.core.tracklist.append([ Track(uri='dummy:a'), Track(uri='dummy:b')]) self.assertEqual(self.core.playback.state.get(), STOPPED) @@ -841,8 +838,7 @@ class PlayerInterfaceTest(unittest.TestCase): def test_open_uri_starts_playback_of_new_track_if_paused(self): self.mpris.get_CanPlay = lambda *_: True - self.backend.library.dummy_library = [ - Track(uri='dummy:/test/uri')] + self.backend.library.dummy_library = [Track(uri='dummy:/test/uri')] self.core.tracklist.append([ Track(uri='dummy:a'), Track(uri='dummy:b')]) self.core.playback.play() @@ -858,8 +854,7 @@ class PlayerInterfaceTest(unittest.TestCase): def test_open_uri_starts_playback_of_new_track_if_playing(self): self.mpris.get_CanPlay = lambda *_: True - self.backend.library.dummy_library = [ - Track(uri='dummy:/test/uri')] + self.backend.library.dummy_library = [Track(uri='dummy:/test/uri')] self.core.tracklist.append([ Track(uri='dummy:a'), Track(uri='dummy:b')]) self.core.playback.play() diff --git a/tests/models_test.py b/tests/models_test.py index 21ad7ead..9a3062fc 100644 --- a/tests/models_test.py +++ b/tests/models_test.py @@ -79,6 +79,13 @@ class ArtistTest(unittest.TestCase): test = lambda: Artist(foo='baz') self.assertRaises(TypeError, test) + def test_invalid_kwarg_with_name_matching_method(self): + test = lambda: Artist(copy='baz') + self.assertRaises(TypeError, test) + + test = lambda: Artist(serialize='baz') + self.assertRaises(TypeError, test) + def test_repr(self): self.assertEquals( "Artist(name=u'name', uri=u'uri')", @@ -86,15 +93,36 @@ class ArtistTest(unittest.TestCase): def test_serialize(self): self.assertDictEqual( - {'__type__': 'Artist', 'uri': 'uri', 'name': 'name'}, + {'__model__': 'Artist', 'uri': 'uri', 'name': 'name'}, Artist(uri='uri', name='name').serialize()) - def test_to_json_and_Back(self): + def test_to_json_and_back(self): artist1 = Artist(uri='uri', name='name') serialized = json.dumps(artist1, cls=ModelJSONEncoder) artist2 = json.loads(serialized, object_hook=model_json_decoder) self.assertEqual(artist1, artist2) + def test_to_json_and_back_with_unknown_field(self): + artist = Artist(uri='uri', name='name').serialize() + artist['foo'] = 'foo' + serialized = json.dumps(artist) + test = lambda: json.loads(serialized, object_hook=model_json_decoder) + self.assertRaises(TypeError, test) + + def test_to_json_and_back_with_field_matching_method(self): + artist = Artist(uri='uri', name='name').serialize() + artist['copy'] = 'foo' + serialized = json.dumps(artist) + test = lambda: json.loads(serialized, object_hook=model_json_decoder) + self.assertRaises(TypeError, test) + + def test_to_json_and_back_with_field_matching_internal_field(self): + artist = Artist(uri='uri', name='name').serialize() + artist['__mro__'] = 'foo' + serialized = json.dumps(artist) + test = lambda: json.loads(serialized, object_hook=model_json_decoder) + self.assertRaises(TypeError, test) + def test_eq_name(self): artist1 = Artist(name='name') artist2 = Artist(name='name') @@ -204,14 +232,14 @@ class AlbumTest(unittest.TestCase): def test_serialize_without_artists(self): self.assertDictEqual( - {'__type__': 'Album', 'uri': 'uri', 'name': 'name'}, + {'__model__': 'Album', 'uri': 'uri', 'name': 'name'}, Album(uri='uri', name='name').serialize()) def test_serialize_with_artists(self): artist = Artist(name='foo') self.assertDictEqual( - {'__type__': 'Album', 'uri': 'uri', 'name': 'name', 'artists': - [artist.serialize()]}, + {'__model__': 'Album', 'uri': 'uri', 'name': 'name', + 'artists': [artist.serialize()]}, Album(uri='uri', name='name', artists=[artist]).serialize()) def test_to_json_and_back(self): @@ -402,20 +430,20 @@ class TrackTest(unittest.TestCase): def test_serialize_without_artists(self): self.assertDictEqual( - {'__type__': 'Track', 'uri': 'uri', 'name': 'name'}, + {'__model__': 'Track', 'uri': 'uri', 'name': 'name'}, Track(uri='uri', name='name').serialize()) def test_serialize_with_artists(self): artist = Artist(name='foo') self.assertDictEqual( - {'__type__': 'Track', 'uri': 'uri', 'name': 'name', + {'__model__': 'Track', 'uri': 'uri', 'name': 'name', 'artists': [artist.serialize()]}, Track(uri='uri', name='name', artists=[artist]).serialize()) def test_serialize_with_album(self): album = Album(name='foo') self.assertDictEqual( - {'__type__': 'Track', 'uri': 'uri', 'name': 'name', + {'__model__': 'Track', 'uri': 'uri', 'name': 'name', 'album': album.serialize()}, Track(uri='uri', name='name', album=album).serialize()) @@ -618,7 +646,7 @@ class TlTrackTest(unittest.TestCase): def test_serialize(self): track = Track(uri='uri', name='name') self.assertDictEqual( - {'__type__': 'TlTrack', 'tlid': 123, 'track': track.serialize()}, + {'__model__': 'TlTrack', 'tlid': 123, 'track': track.serialize()}, TlTrack(tlid=123, track=track).serialize()) def test_to_json_and_back(self): @@ -752,13 +780,13 @@ class PlaylistTest(unittest.TestCase): def test_serialize_without_tracks(self): self.assertDictEqual( - {'__type__': 'Playlist', 'uri': 'uri', 'name': 'name'}, + {'__model__': 'Playlist', 'uri': 'uri', 'name': 'name'}, Playlist(uri='uri', name='name').serialize()) def test_serialize_with_tracks(self): track = Track(name='foo') self.assertDictEqual( - {'__type__': 'Playlist', 'uri': 'uri', 'name': 'name', + {'__model__': 'Playlist', 'uri': 'uri', 'name': 'name', 'tracks': [track.serialize()]}, Playlist(uri='uri', name='name', tracks=[track]).serialize())