From 88d64044dec008b075654e2c97273d92f0ef8d9b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 22 Sep 2014 21:42:57 +0200 Subject: [PATCH 01/18] models: Hide empty lists from repr() (cherry picked from commit 305a76486d59a4fd24f335b16c80854dd0a232df) Conflicts: docs/changelog.rst --- docs/changelog.rst | 6 ++++++ mopidy/models.py | 2 ++ tests/test_models.py | 21 ++++++++------------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index ad707948..c0a1a9b8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,6 +5,12 @@ Changelog This changelog is used to track all major changes to Mopidy. +v0.19.5 (UNRELEASED) +==================== + +- Models: Hide empty collections from :func:`repr()` representations. + + v0.19.4 (2014-09-01) ==================== diff --git a/mopidy/models.py b/mopidy/models.py index 42313922..83888ae5 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -29,6 +29,8 @@ class ImmutableObject(object): kwarg_pairs = [] for (key, value) in sorted(self.__dict__.items()): if isinstance(value, (frozenset, tuple)): + if not value: + continue value = list(value) kwarg_pairs.append('%s=%s' % (key, repr(value))) return '%(classname)s(%(kwargs)s)' % { diff --git a/tests/test_models.py b/tests/test_models.py index 43343ce7..448d6208 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -318,13 +318,12 @@ class AlbumTest(unittest.TestCase): def test_repr_without_artists(self): self.assertEquals( - "Album(artists=[], images=[], name=u'name', uri=u'uri')", + "Album(name=u'name', uri=u'uri')", repr(Album(uri='uri', name='name'))) def test_repr_with_artists(self): self.assertEquals( - "Album(artists=[Artist(name=u'foo')], images=[], name=u'name', " - "uri=u'uri')", + "Album(artists=[Artist(name=u'foo')], name=u'name', uri=u'uri')", repr(Album(uri='uri', name='name', artists=[Artist(name='foo')]))) def test_serialize_without_artists(self): @@ -551,14 +550,12 @@ class TrackTest(unittest.TestCase): def test_repr_without_artists(self): self.assertEquals( - "Track(artists=[], composers=[], name=u'name', " - "performers=[], uri=u'uri')", + "Track(name=u'name', uri=u'uri')", repr(Track(uri='uri', name='name'))) def test_repr_with_artists(self): self.assertEquals( - "Track(artists=[Artist(name=u'foo')], composers=[], name=u'name', " - "performers=[], uri=u'uri')", + "Track(artists=[Artist(name=u'foo')], name=u'name', uri=u'uri')", repr(Track(uri='uri', name='name', artists=[Artist(name='foo')]))) def test_serialize_without_artists(self): @@ -773,8 +770,7 @@ class TlTrackTest(unittest.TestCase): def test_repr(self): self.assertEquals( - "TlTrack(tlid=123, track=Track(artists=[], composers=[], " - "performers=[], uri=u'uri'))", + "TlTrack(tlid=123, track=Track(uri=u'uri'))", repr(TlTrack(tlid=123, track=Track(uri='uri')))) def test_serialize(self): @@ -903,13 +899,12 @@ class PlaylistTest(unittest.TestCase): def test_repr_without_tracks(self): self.assertEquals( - "Playlist(name=u'name', tracks=[], uri=u'uri')", + "Playlist(name=u'name', uri=u'uri')", repr(Playlist(uri='uri', name='name'))) def test_repr_with_tracks(self): self.assertEquals( - "Playlist(name=u'name', tracks=[Track(artists=[], composers=[], " - "name=u'foo', performers=[])], uri=u'uri')", + "Playlist(name=u'name', tracks=[Track(name=u'foo')], uri=u'uri')", repr(Playlist(uri='uri', name='name', tracks=[Track(name='foo')]))) def test_serialize_without_tracks(self): @@ -1036,7 +1031,7 @@ class SearchResultTest(unittest.TestCase): def test_repr_without_results(self): self.assertEquals( - "SearchResult(albums=[], artists=[], tracks=[], uri=u'uri')", + "SearchResult(uri=u'uri')", repr(SearchResult(uri='uri'))) def test_serialize_without_results(self): From a4b17a9aa81afcb7041c92544ad55a6cfed60ef2 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 22 Sep 2014 21:48:26 +0200 Subject: [PATCH 02/18] models: Fix equality for fields set to the default Fixes #837 (cherry picked from commit bdd1fb983b3abbbb604686e63647a8aaaa5d97a0) --- docs/changelog.rst | 6 ++++++ mopidy/models.py | 8 ++++---- tests/test_models.py | 22 ++++++++++++++++++++-- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c0a1a9b8..bde9759b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,6 +10,12 @@ v0.19.5 (UNRELEASED) - Models: Hide empty collections from :func:`repr()` representations. +- Field values are no longer stored on the model instance when the value + matches the default value for the field. This makes two models equal when + they have a field which in one case is implicitly set to the default value + and in the other case explicitly set to the default value, but with otherwise + equal fields. (Fixes: :issue:`837`) + v0.19.4 (2014-09-01) ==================== diff --git a/mopidy/models.py b/mopidy/models.py index 83888ae5..510cb56a 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -18,6 +18,8 @@ class ImmutableObject(object): raise TypeError( '__init__() got an unexpected keyword argument "%s"' % key) + if value == getattr(self, key): + continue # Don't explicitly set default values self.__dict__[key] = value def __setattr__(self, name, value): @@ -72,13 +74,11 @@ class ImmutableObject(object): for key in self.__dict__.keys(): public_key = key.lstrip('_') value = values.pop(public_key, self.__dict__[key]) - if value is not None: - data[public_key] = value + data[public_key] = value for key in values.keys(): if hasattr(self, key): value = values.pop(key) - if value is not None: - data[key] = value + data[key] = value if values: raise TypeError( 'copy() got an unexpected keyword argument "%s"' % key) diff --git a/tests/test_models.py b/tests/test_models.py index 448d6208..09610b99 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -171,8 +171,8 @@ class ArtistTest(unittest.TestCase): def test_serialize_falsy_values(self): self.assertDictEqual( - {'__model__': 'Artist', 'uri': '', 'name': None}, - Artist(uri='', name=None).serialize()) + {'__model__': 'Artist', 'uri': '', 'name': ''}, + Artist(uri='', name='').serialize()) def test_to_json_and_back(self): artist1 = Artist(uri='uri', name='name') @@ -735,6 +735,24 @@ class TrackTest(unittest.TestCase): self.assertNotEqual(track1, track2) self.assertNotEqual(hash(track1), hash(track2)) + def test_ignores_values_with_default_value_none(self): + track1 = Track(name='name1') + track2 = Track(name='name1', album=None) + self.assertEqual(track1, track2) + self.assertEqual(hash(track1), hash(track2)) + + def test_ignores_values_with_default_value_zero(self): + track1 = Track(name='name1') + track2 = Track(name='name1', track_no=0) + self.assertEqual(track1, track2) + self.assertEqual(hash(track1), hash(track2)) + + def test_copy_can_reset_to_default_value(self): + track1 = Track(name='name1') + track2 = Track(name='name1', album=Album()).copy(album=None) + self.assertEqual(track1, track2) + self.assertEqual(hash(track1), hash(track2)) + class TlTrackTest(unittest.TestCase): def test_tlid(self): From 680dbffc0c149d1d71b940678124f4dd571d90e6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 22 Sep 2014 22:25:42 +0200 Subject: [PATCH 03/18] models: Make all fields default to None or empty collection (cherry picked from commit abed15b9e409aaa5f50a689012d875b0903313ea) --- docs/changelog.rst | 4 ++++ mopidy/models.py | 12 ++++++------ mopidy/mpd/translator.py | 6 +++--- tests/test_models.py | 6 ------ 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index bde9759b..c296e557 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,10 @@ v0.19.5 (UNRELEASED) and in the other case explicitly set to the default value, but with otherwise equal fields. (Fixes: :issue:`837`) +- Changed the default value of :attr:`mopidy.models.Album.num_tracks`, + :attr:`mopidy.models.Track.track_no`, and + :attr:`mopidy.models.Track.last_modified` from ``0`` to :class:`None`. + v0.19.4 (2014-09-01) ==================== diff --git a/mopidy/models.py b/mopidy/models.py index 510cb56a..bedf8ca5 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -241,7 +241,7 @@ class Album(ImmutableObject): :param artists: album artists :type artists: list of :class:`Artist` :param num_tracks: number of tracks in album - :type num_tracks: integer + :type num_tracks: integer or :class:`None` if unknown :param num_discs: number of discs in album :type num_discs: integer or :class:`None` if unknown :param date: album release date (YYYY or YYYY-MM-DD) @@ -262,7 +262,7 @@ class Album(ImmutableObject): artists = frozenset() #: The number of tracks in the album. Read-only. - num_tracks = 0 + num_tracks = None #: The number of discs in the album. Read-only. num_discs = None @@ -302,7 +302,7 @@ class Track(ImmutableObject): :param genre: track genre :type genre: string :param track_no: track number in album - :type track_no: integer + :type track_no: integer or :class:`None` if unknown :param disc_no: disc number in album :type disc_no: integer or :class:`None` if unknown :param date: track release date (YYYY or YYYY-MM-DD) @@ -316,7 +316,7 @@ class Track(ImmutableObject): :param musicbrainz_id: MusicBrainz ID :type musicbrainz_id: string :param last_modified: Represents last modification time - :type last_modified: integer + :type last_modified: integer or :class:`None` if unknown """ #: The track URI. Read-only. @@ -341,7 +341,7 @@ class Track(ImmutableObject): genre = None #: The track number in the album. Read-only. - track_no = 0 + track_no = None #: The disc number in the album. Read-only. disc_no = None @@ -364,7 +364,7 @@ class Track(ImmutableObject): #: Integer representing when the track was last modified, exact meaning #: depends on source of track. For local files this is the mtime, for other #: backends it could be a timestamp or simply a version counter. - last_modified = 0 + last_modified = None def __init__(self, *args, **kwargs): get = lambda key: frozenset(kwargs.pop(key, None) or []) diff --git a/mopidy/mpd/translator.py b/mopidy/mpd/translator.py index 252725ee..f3264a46 100644 --- a/mopidy/mpd/translator.py +++ b/mopidy/mpd/translator.py @@ -44,11 +44,11 @@ def track_to_mpd_format(track, position=None): if track.date: result.append(('Date', track.date)) - if track.album is not None and track.album.num_tracks != 0: + if track.album is not None and track.album.num_tracks is not None: result.append(('Track', '%d/%d' % ( - track.track_no, track.album.num_tracks))) + track.track_no or 0, track.album.num_tracks))) else: - result.append(('Track', track.track_no)) + result.append(('Track', track.track_no or 0)) if position is not None and tlid is not None: result.append(('Pos', position)) result.append(('Id', tlid)) diff --git a/tests/test_models.py b/tests/test_models.py index 09610b99..56d6c76b 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -741,12 +741,6 @@ class TrackTest(unittest.TestCase): self.assertEqual(track1, track2) self.assertEqual(hash(track1), hash(track2)) - def test_ignores_values_with_default_value_zero(self): - track1 = Track(name='name1') - track2 = Track(name='name1', track_no=0) - self.assertEqual(track1, track2) - self.assertEqual(hash(track1), hash(track2)) - def test_copy_can_reset_to_default_value(self): track1 = Track(name='name1') track2 = Track(name='name1', album=Album()).copy(album=None) From c070c1c0b5c9941c538f106cb7051f9fbcff8a05 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 4 Nov 2014 21:27:59 +0100 Subject: [PATCH 04/18] docs: Update changelog --- docs/changelog.rst | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c296e557..bddad372 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,15 +8,17 @@ This changelog is used to track all major changes to Mopidy. v0.19.5 (UNRELEASED) ==================== +Bug fix release. + - Models: Hide empty collections from :func:`repr()` representations. -- Field values are no longer stored on the model instance when the value - matches the default value for the field. This makes two models equal when - they have a field which in one case is implicitly set to the default value - and in the other case explicitly set to the default value, but with otherwise - equal fields. (Fixes: :issue:`837`) +- Models: Field values are no longer stored on the model instance when the + value matches the default value for the field. This makes two models equal + when they have a field which in one case is implicitly set to the default + value and in the other case explicitly set to the default value, but with + otherwise equal fields. (Fixes: :issue:`837`) -- Changed the default value of :attr:`mopidy.models.Album.num_tracks`, +- Models: Changed the default value of :attr:`mopidy.models.Album.num_tracks`, :attr:`mopidy.models.Track.track_no`, and :attr:`mopidy.models.Track.last_modified` from ``0`` to :class:`None`. From 4a6e7d292c7d5fdbf7b30b8f4363d023f072ac87 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 25 Nov 2014 21:37:55 +0100 Subject: [PATCH 05/18] docs: Don't refer to the tracklist as a playlist --- mopidy/core/tracklist.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 1bce8734..4c567086 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -77,9 +77,9 @@ class TracklistController(object): consume = property(get_consume, set_consume) """ :class:`True` - Tracks are removed from the playlist when they have been played. + Tracks are removed from the tracklist when they have been played. :class:`False` - Tracks are not removed from the playlist. + Tracks are not removed from the tracklist. """ def get_random(self): @@ -96,9 +96,9 @@ class TracklistController(object): random = property(get_random, set_random) """ :class:`True` - Tracks are selected at random from the playlist. + Tracks are selected at random from the tracklist. :class:`False` - Tracks are played in the order of the playlist. + Tracks are played in the order of the tracklist. """ def get_repeat(self): @@ -112,10 +112,10 @@ class TracklistController(object): repeat = property(get_repeat, set_repeat) """ :class:`True` - The current playlist is played repeatedly. To repeat a single track, - select both :attr:`repeat` and :attr:`single`. + The tracklist is played repeatedly. To repeat a single track, select + both :attr:`repeat` and :attr:`single`. :class:`False` - The current playlist is played once. + The tracklist is played once. """ def get_single(self): @@ -175,10 +175,10 @@ class TracklistController(object): The track that will be played if calling :meth:`mopidy.core.PlaybackController.next()`. - 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 + For normal playback this is the next track in the tracklist. If repeat + is enabled the next track can loop around the tracklist. When random is enabled this should be a random track, all tracks should be played once - before the list repeats. + before the tracklist repeats. :param tl_track: the reference track :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None` @@ -217,7 +217,7 @@ class TracklistController(object): Returns the track that will be played if calling :meth:`mopidy.core.PlaybackController.previous()`. - For normal playback this is the previous track in the playlist. If + For normal playback this is the previous track in the tracklist. If random and/or consume is enabled it should return the current track instead. From 6e55435aa90fbeb2d4d8e3906c8e56414b03aa9e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 13 Dec 2014 01:26:41 +0100 Subject: [PATCH 06/18] mpd: Enable browsing of empty dirs This was disabled together with a bunch of other changes without any explanation in commit f24ca36e5a5b72c886d6d8e8df88be79fb094dda. I'm guessing that this wasn't intentional, and no test covered the case. (cherry picked from commit 4e508cd01750bcef9e0e9c7b70798eacd276aef8) --- mopidy/mpd/protocol/music_db.py | 2 -- tests/mpd/protocol/test_music_db.py | 7 +++++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index a5757915..5a7f51fb 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -421,8 +421,6 @@ def lsinfo(context, uri=None): if uri in (None, '', '/'): result.extend(protocol.stored_playlists.listplaylists(context)) - if not result: - raise exceptions.MpdNoExistError('Not found') return result diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index e6712fef..8efd1dc4 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -347,6 +347,13 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertInResponse('directory: dummy/foo') self.assertInResponse('OK') + def test_lsinfo_for_empty_dir_returns_nothing(self): + self.backend.library.dummy_browse_result = { + 'dummy:/': []} + + self.sendRequest('lsinfo "/dummy"') + self.assertInResponse('OK') + def test_lsinfo_for_dir_does_not_recurse(self): self.backend.library.dummy_library = [ Track(uri='dummy:/a', name='a'), From b365d2494bceb84c328c2f6888bb139ba3b43376 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 14 Dec 2014 14:16:37 +0100 Subject: [PATCH 07/18] mpd: Remove "Comment" tag type from translator output. Newer versions of the protocol have removed this tag, so we should as well. This also works around the issue of #881 which was breaking things with newlines in comment fields. The readcomments command seems to replace this, but it seems to only care about specific extra tagtypes, not the general comment tag we normally collect when scanning things. (cherry picked from commit 08a8d5c43be271a41af74ce0c7df9d871cf2b532) --- mopidy/mpd/translator.py | 3 --- tests/mpd/test_translator.py | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/mopidy/mpd/translator.py b/mopidy/mpd/translator.py index f3264a46..8a43f929 100644 --- a/mopidy/mpd/translator.py +++ b/mopidy/mpd/translator.py @@ -81,9 +81,6 @@ def track_to_mpd_format(track, position=None): if track.disc_no: result.append(('Disc', track.disc_no)) - if track.comment: - result.append(('Comment', track.comment)) - if track.musicbrainz_id is not None: result.append(('MUSICBRAINZ_TRACKID', track.musicbrainz_id)) return result diff --git a/tests/mpd/test_translator.py b/tests/mpd/test_translator.py index 2bd6cff6..05a0d187 100644 --- a/tests/mpd/test_translator.py +++ b/tests/mpd/test_translator.py @@ -73,10 +73,10 @@ class TrackMpdFormatTest(unittest.TestCase): self.assertIn(('Track', '7/13'), result) self.assertIn(('Date', datetime.date(1977, 1, 1)), result) self.assertIn(('Disc', '1'), result) - self.assertIn(('Comment', 'a comment'), result) self.assertIn(('Pos', 9), result) self.assertIn(('Id', 122), result) - self.assertEqual(len(result), 15) + self.assertNotIn(('Comment', 'a comment'), result) + self.assertEqual(len(result), 14) def test_track_to_mpd_format_musicbrainz_trackid(self): track = self.track.copy(musicbrainz_id='foo') From d2bf3f6d830fb5d4f929af324052f1699c82fcb7 Mon Sep 17 00:00:00 2001 From: Thomas Amland Date: Thu, 11 Dec 2014 14:41:37 +0100 Subject: [PATCH 08/18] [local] fix modified files not being updated (cherry picked from commit dfd897832a952c4a7da655f504dc0c162433c539) --- mopidy/local/commands.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mopidy/local/commands.py b/mopidy/local/commands.py index 1e7839a5..a0dc23d1 100644 --- a/mopidy/local/commands.py +++ b/mopidy/local/commands.py @@ -70,7 +70,6 @@ class ScanCommand(commands.Command): library = _get_library(args, config) - uris_in_library = set() uris_to_update = set() uris_to_remove = set() @@ -87,7 +86,7 @@ class ScanCommand(commands.Command): logger.debug('Missing file %s', track.uri) uris_to_remove.add(track.uri) elif mtime > track.last_modified: - uris_in_library.add(track.uri) + uris_to_update.add(track.uri) logger.info('Removing %d missing tracks.', len(uris_to_remove)) for uri in uris_to_remove: From 188c9ef26fd193b7e913abb9997ebe87fd1dbeca Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 16 Dec 2014 23:34:00 +0100 Subject: [PATCH 09/18] docs: Update changelog --- docs/changelog.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index bddad372..979a5a0c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -22,6 +22,13 @@ Bug fix release. :attr:`mopidy.models.Track.track_no`, and :attr:`mopidy.models.Track.last_modified` from ``0`` to :class:`None`. +- Local: Fix scanning of modified files. (PR: :issue:`904`) + +- MPD: Re-enable browsing of empty directories. (PR: :issue:`906`) + +- MPD: Remove track comments from responses. They are not included by the + original MPD server, and this works around :issue:`881`. (PR: :issue:`882`) + v0.19.4 (2014-09-01) ==================== From b300c090341450a5337c434ab6c521733f973b4e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 16 Dec 2014 23:35:42 +0100 Subject: [PATCH 10/18] docs: Update authors --- .mailmap | 1 + AUTHORS | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/.mailmap b/.mailmap index d380162e..45935be6 100644 --- a/.mailmap +++ b/.mailmap @@ -16,3 +16,4 @@ Janez Troha Luke Giuliani Colin Montgomerie Ignasi Fosch +Christopher Schirner diff --git a/AUTHORS b/AUTHORS index e36d953d..b7bad761 100644 --- a/AUTHORS +++ b/AUTHORS @@ -42,3 +42,7 @@ - Sam Willcocks - Ignasi Fosch - Arjun Naik +- Christopher Schirner +- Dmitry Sandalov +- Deni Bertovic +- Thomas Amland From 94bdb88b9cb161dd9736c74c1804c03246d235a3 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 20 Dec 2014 21:32:08 +0100 Subject: [PATCH 11/18] http: Log errors instead of dying for HTTP startup. (cherry picked from commit 9a2f8a3e4f5ea80ba0e6c497acfc8bee40a04375) Conflicts: docs/changelog.rst --- docs/changelog.rst | 3 +++ mopidy/http/actor.py | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 979a5a0c..ba00ed1f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -29,6 +29,9 @@ Bug fix release. - MPD: Remove track comments from responses. They are not included by the original MPD server, and this works around :issue:`881`. (PR: :issue:`882`) +- HTTP: Errors while starting HTTP apps are logged instead of crashing the HTTP + server. (Fixes: :issue:`875`) + v0.19.4 (2014-09-01) ==================== diff --git a/mopidy/http/actor.py b/mopidy/http/actor.py index 57e2f46a..a61fc45c 100644 --- a/mopidy/http/actor.py +++ b/mopidy/http/actor.py @@ -129,11 +129,16 @@ class HttpServer(threading.Thread): def _get_app_request_handlers(self): result = [] for app in self.apps: + try: + request_handlers = app['factory'](self.config, self.core) + except Exception: + logger.exception('Loading %s failed.', app['name']) + continue + result.append(( r'/%s' % app['name'], handlers.AddSlashHandler )) - request_handlers = app['factory'](self.config, self.core) for handler in request_handlers: handler = list(handler) handler[0] = '/%s%s' % (app['name'], handler[0]) From 532905bd7440ec7663110db5b6fa0c86f825ff28 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 23 Dec 2014 22:20:49 +0100 Subject: [PATCH 12/18] docs: Add :discuss: shortcut for linking to forum threads --- docs/conf.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/conf.py b/docs/conf.py index 52e84e06..f7475293 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -156,6 +156,7 @@ extlinks = { 'commit': ('https://github.com/mopidy/mopidy/commit/%s', 'commit '), 'mpris': ( 'https://github.com/mopidy/mopidy-mpris/issues/%s', 'mopidy-mpris#'), + 'discuss': ('https://discuss.mopidy.com/t/%s', 'discuss.mopidy.com/t/'), } From aa3b8ab5f84798428134d1dda2c944e98879a168 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 23 Dec 2014 22:11:11 +0100 Subject: [PATCH 13/18] path: Support unicode content when creating file --- mopidy/utils/path.py | 6 ++++-- tests/utils/test_path.py | 12 +++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/mopidy/utils/path.py b/mopidy/utils/path.py index 5870fc6e..7c1f671b 100644 --- a/mopidy/utils/path.py +++ b/mopidy/utils/path.py @@ -44,12 +44,14 @@ def get_or_create_file(file_path, mkdir=True, content=None): if not isinstance(file_path, bytes): raise ValueError('Path is not a bytestring.') file_path = expand_path(file_path) + if isinstance(content, unicode): + content = content.encode('utf-8') if mkdir: get_or_create_dir(os.path.dirname(file_path)) if not os.path.isfile(file_path): logger.info('Creating file %s', file_path) - with open(file_path, 'w') as fh: - if content: + with open(file_path, 'wb') as fh: + if content is not None: fh.write(content) return file_path diff --git a/tests/utils/test_path.py b/tests/utils/test_path.py index 078cdb20..db152654 100644 --- a/tests/utils/test_path.py +++ b/tests/utils/test_path.py @@ -105,12 +105,12 @@ class GetOrCreateFileTest(unittest.TestCase): with self.assertRaises(IOError): path.get_or_create_file(conflicting_dir) - def test_create_dir_with_unicode(self): + def test_create_dir_with_unicode_filename_throws_value_error(self): with self.assertRaises(ValueError): file_path = unicode(os.path.join(self.parent, b'test')) path.get_or_create_file(file_path) - def test_create_file_with_none(self): + def test_create_file_with_none_filename_throws_value_error(self): with self.assertRaises(ValueError): path.get_or_create_file(None) @@ -119,12 +119,18 @@ class GetOrCreateFileTest(unittest.TestCase): with self.assertRaises(IOError): path.get_or_create_file(file_path, mkdir=False) - def test_create_dir_with_default_content(self): + def test_create_dir_with_bytes_content(self): file_path = os.path.join(self.parent, b'test') created = path.get_or_create_file(file_path, content=b'foobar') with open(created) as fh: self.assertEqual(fh.read(), b'foobar') + def test_create_dir_with_unicode_content(self): + file_path = os.path.join(self.parent, b'test') + created = path.get_or_create_file(file_path, content='foobaræøå') + with open(created) as fh: + self.assertEqual(fh.read(), b'foobaræøå') + class PathToFileURITest(unittest.TestCase): def test_simple_path(self): From fcf39833ca33a107954f052cbaedcf988689d43a Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 23 Dec 2014 22:12:34 +0100 Subject: [PATCH 14/18] config: Support UTF-8 in default config Fixes issue reported at https://discuss.mopidy.com/t/428. Mopidy-HTTP-Kuechenradio includes a non-ASCII UTF-8 character in its default config. If Mopidy didn't already have a config file, it crashed when trying to create the initial config file based on the default config of all available extensions. --- docs/changelog.rst | 6 ++++++ mopidy/config/__init__.py | 8 ++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index ba00ed1f..1ef13666 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,6 +10,12 @@ v0.19.5 (UNRELEASED) Bug fix release. +- config: Support UTF-8 in default config. If an extension with non-ASCII + characters in its default config was installed, and Mopidy didn't already + have a config file, Mopidy would crashed when trying to create the initial + config file based on the default config of all available extensions. + (Fixes: :discuss:`428`) + - Models: Hide empty collections from :func:`repr()` representations. - Models: Field values are no longer stored on the model instance when the diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 5611c717..a6578825 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -97,8 +97,12 @@ def format_initial(extensions): versions = ['Mopidy %s' % versioning.get_version()] for extension in sorted(extensions, key=lambda ext: ext.dist_name): versions.append('%s %s' % (extension.dist_name, extension.version)) - description = _INITIAL_HELP.strip() % {'versions': '\n# '.join(versions)} - return description + '\n\n' + _format(config, {}, schemas, False, True) + + header = _INITIAL_HELP.strip() % {'versions': '\n# '.join(versions)} + formatted_config = _format( + config=config, comments={}, schemas=schemas, + display=False, disable=True).decode('utf-8') + return header + '\n\n' + formatted_config def _load(files, defaults, overrides): From 302bb7c2218331d8de32375138794060bb9e5b4c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 23 Dec 2014 22:58:17 +0100 Subject: [PATCH 15/18] ext: Fix unpacking of VersionConflict exception Fixes #911 --- docs/changelog.rst | 6 +++++- mopidy/ext.py | 11 +++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 1ef13666..19257e46 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,12 +10,16 @@ v0.19.5 (UNRELEASED) Bug fix release. -- config: Support UTF-8 in default config. If an extension with non-ASCII +- Config: Support UTF-8 in default config. If an extension with non-ASCII characters in its default config was installed, and Mopidy didn't already have a config file, Mopidy would crashed when trying to create the initial config file based on the default config of all available extensions. (Fixes: :discuss:`428`) +- Extensions: Fix crash when unpacking data from + :exc:`pkg_resources.VersionConflict` created with a single argument. (Fixes: + :issue:`911`) + - Models: Hide empty collections from :func:`repr()` representations. - Models: Field values are no longer stored on the model instance when the diff --git a/mopidy/ext.py b/mopidy/ext.py index 3333ec3f..24d85786 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -188,10 +188,13 @@ def validate_extension(extension): extension.ext_name, ex) return False except pkg_resources.VersionConflict as ex: - found, required = ex.args - logger.info( - 'Disabled extension %s: %s required, but found %s at %s', - extension.ext_name, required, found, found.location) + if len(ex.args) == 2: + found, required = ex.args + logger.info( + 'Disabled extension %s: %s required, but found %s at %s', + extension.ext_name, required, found, found.location) + else: + logger.info('Disabled extension %s: %s', extension.ext_name, ex) return False try: From c6ff9eee86404aae2dc8afd28e7b9481eaf98228 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 23 Dec 2014 23:58:20 +0100 Subject: [PATCH 16/18] playback: Remove skipped track on next in consume mode Also adds core level tests of consume behavior on next/prev/eot. Fixes #902 --- docs/changelog.rst | 4 +++ mopidy/core/playback.py | 10 ++++--- mopidy/core/tracklist.py | 8 +++--- tests/core/test_playback.py | 51 ++++++++++++++++++++++++++++++++++++ tests/local/test_playback.py | 2 +- 5 files changed, 67 insertions(+), 8 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 19257e46..4e3662f1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -32,6 +32,10 @@ Bug fix release. :attr:`mopidy.models.Track.track_no`, and :attr:`mopidy.models.Track.last_modified` from ``0`` to :class:`None`. +- Core: When skipping to the next track in consume mode, remove the skipped + track from the tracklist. This is consistent with the original MPD server's + behavior. (Fixes: :issue:`902`) + - Local: Fix scanning of modified files. (PR: :issue:`904`) - MPD: Re-enable browsing of empty directories. (PR: :issue:`906`) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 097a9401..d6307e2f 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -179,12 +179,16 @@ class PlaybackController(object): The current playback state will be kept. If it was playing, playing will continue. If it was paused, it will still be paused, etc. """ - tl_track = self.core.tracklist.next_track(self.current_tl_track) - if tl_track: - self.change_track(tl_track) + original_tl_track = self.current_tl_track + next_tl_track = self.core.tracklist.next_track(original_tl_track) + + if next_tl_track: + self.change_track(next_tl_track) else: self.stop(clear_current_track=True) + self.core.tracklist.mark_played(original_tl_track) + def pause(self): """Pause playback.""" backend = self._get_backend() diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 4c567086..80ed5aea 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -448,10 +448,10 @@ class TracklistController(object): def mark_played(self, tl_track): """Private method used by :class:`mopidy.core.PlaybackController`.""" - if not self.consume: - return False - self.remove(tlid=[tl_track.tlid]) - return True + if self.consume and tl_track is not None: + self.remove(tlid=[tl_track.tlid]) + return True + return False def _trigger_tracklist_changed(self): if self.random: diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index ce6c8571..909ca4fa 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -239,6 +239,23 @@ class CorePlaybackTest(unittest.TestCase): # TODO Test next() more + def test_next_keeps_finished_track_in_tracklist(self): + tl_track = self.tl_tracks[0] + self.core.playback.play(tl_track) + + self.core.playback.next() + + self.assertIn(tl_track, self.core.tracklist.tl_tracks) + + def test_next_in_consume_mode_removes_finished_track(self): + tl_track = self.tl_tracks[0] + self.core.playback.play(tl_track) + self.core.tracklist.consume = True + + self.core.playback.next() + + self.assertNotIn(tl_track, self.core.tracklist.tl_tracks) + @mock.patch( 'mopidy.core.playback.listener.CoreListener', spec=core.CoreListener) def test_next_emits_events(self, listener_mock): @@ -265,6 +282,23 @@ class CorePlaybackTest(unittest.TestCase): # TODO Test previous() more + def test_previous_keeps_finished_track_in_tracklist(self): + tl_track = self.tl_tracks[1] + self.core.playback.play(tl_track) + + self.core.playback.previous() + + self.assertIn(tl_track, self.core.tracklist.tl_tracks) + + def test_previous_keeps_finished_track_even_in_consume_mode(self): + tl_track = self.tl_tracks[1] + self.core.playback.play(tl_track) + self.core.tracklist.consume = True + + self.core.playback.previous() + + self.assertIn(tl_track, self.core.tracklist.tl_tracks) + @mock.patch( 'mopidy.core.playback.listener.CoreListener', spec=core.CoreListener) def test_previous_emits_events(self, listener_mock): @@ -291,6 +325,23 @@ class CorePlaybackTest(unittest.TestCase): # TODO Test on_end_of_track() more + def test_on_end_of_track_keeps_finished_track_in_tracklist(self): + tl_track = self.tl_tracks[0] + self.core.playback.play(tl_track) + + self.core.playback.on_end_of_track() + + self.assertIn(tl_track, self.core.tracklist.tl_tracks) + + def test_on_end_of_track_in_consume_mode_removes_finished_track(self): + tl_track = self.tl_tracks[0] + self.core.playback.play(tl_track) + self.core.tracklist.consume = True + + self.core.playback.on_end_of_track() + + self.assertNotIn(tl_track, self.core.tracklist.tl_tracks) + @mock.patch( 'mopidy.core.playback.listener.CoreListener', spec=core.CoreListener) def test_on_end_of_track_emits_events(self, listener_mock): diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index 7b64e495..89015739 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -347,7 +347,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.tracklist.consume = True self.playback.play() self.playback.next() - self.assertIn(self.tracks[0], self.tracklist.tracks) + self.assertNotIn(self.tracks[0], self.tracklist.tracks) @populate_tracklist def test_next_with_single_and_repeat(self): From 884c35e1f4169a8855a82f99bdb3a46ed4916a65 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 24 Dec 2014 00:36:52 +0100 Subject: [PATCH 17/18] docs: Update changelog --- docs/changelog.rst | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4e3662f1..1b88c489 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,15 +5,16 @@ Changelog This changelog is used to track all major changes to Mopidy. -v0.19.5 (UNRELEASED) +v0.19.5 (2014-12-23) ==================== -Bug fix release. +Today is Mopidy's five year anniversary. We're celebrating with a bugfix +release and are looking forward to the next five years! -- Config: Support UTF-8 in default config. If an extension with non-ASCII - characters in its default config was installed, and Mopidy didn't already - have a config file, Mopidy would crashed when trying to create the initial - config file based on the default config of all available extensions. +- Config: Support UTF-8 in extension's default config. If an extension with + non-ASCII characters in its default config was installed, and Mopidy didn't + already have a config file, Mopidy would crashed when trying to create the + initial config file based on the default config of all available extensions. (Fixes: :discuss:`428`) - Extensions: Fix crash when unpacking data from From 6af624a3699df40369c1b95cff5a4c4eab51adaf Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 24 Dec 2014 00:37:33 +0100 Subject: [PATCH 18/18] Bump version to 0.19.5 --- mopidy/__init__.py | 2 +- tests/test_version.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/mopidy/__init__.py b/mopidy/__init__.py index 7b55f20a..6c122057 100644 --- a/mopidy/__init__.py +++ b/mopidy/__init__.py @@ -21,4 +21,4 @@ if (isinstance(pykka.__version__, basestring) warnings.filterwarnings('ignore', 'could not open display') -__version__ = '0.19.4' +__version__ = '0.19.5' diff --git a/tests/test_version.py b/tests/test_version.py index 96063a1b..7e3922ca 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -50,5 +50,6 @@ class VersionTest(unittest.TestCase): self.assertLess(SV('0.19.0'), SV('0.19.1')) self.assertLess(SV('0.19.1'), SV('0.19.2')) self.assertLess(SV('0.19.2'), SV('0.19.3')) - self.assertLess(SV('0.19.3'), SV(__version__)) - self.assertLess(SV(__version__), SV('0.19.5')) + self.assertLess(SV('0.19.3'), SV('0.19.4')) + self.assertLess(SV('0.19.4'), SV(__version__)) + self.assertLess(SV(__version__), SV('0.19.6'))