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 diff --git a/docs/changelog.rst b/docs/changelog.rst index ad707948..1b88c489 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,6 +5,49 @@ Changelog This changelog is used to track all major changes to Mopidy. +v0.19.5 (2014-12-23) +==================== + +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 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 + :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 + 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: 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`. + +- 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`) + +- 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/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/'), } 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/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): 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/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: 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]) 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: diff --git a/mopidy/models.py b/mopidy/models.py index 42313922..bedf8ca5 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): @@ -29,6 +31,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)' % { @@ -70,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) @@ -239,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) @@ -260,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 @@ -300,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) @@ -314,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. @@ -339,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 @@ -362,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/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/mopidy/mpd/translator.py b/mopidy/mpd/translator.py index 252725ee..8a43f929 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)) @@ -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/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/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): 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'), 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') diff --git a/tests/test_models.py b/tests/test_models.py index 43343ce7..56d6c76b 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') @@ -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): @@ -738,6 +735,18 @@ 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_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): @@ -773,8 +782,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 +911,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 +1043,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): 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')) 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):