Release v0.19.5

.mailmap

@@ -16,3 +16,4 @@ Janez Troha <janez.troha@gmail.com> <dz0ny@ubuntu.si>
 Luke Giuliani <luke@giuliani.com.au>
 Colin Montgomerie <kiteflyingmonkey@gmail.com>
 Ignasi Fosch <natx@y10k.ws> <ifosch@serenity-2.local>
+Christopher Schirner <christopher@hackerspace-bamberg.de> <schinken@hackerspace-bamberg.de>

AUTHORS

@@ -42,3 +42,7 @@
 - Sam Willcocks <sam@wlcx.cc>
 - Ignasi Fosch <natx@y10k.ws>
 - Arjun Naik <arjun@arjunnaik.in>
+- Christopher Schirner <christopher@hackerspace-bamberg.de>
+- Dmitry Sandalov <dmitry@sandalov.org>
+- Deni Bertovic <deni@kset.org>
+- Thomas Amland <thomas.amland@gmail.com>

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)
 ====================
 

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/'),
 }
 
 

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'

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):

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)
+        original_tl_track = self.current_tl_track
-        if tl_track:
+        next_tl_track = self.core.tracklist.next_track(original_tl_track)
-            self.change_track(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()

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:
+        if self.consume and tl_track is not None:
-            return False
+            self.remove(tlid=[tl_track.tlid])
-        self.remove(tlid=[tl_track.tlid])
+            return True
-        return True
+        return False
 
     def _trigger_tracklist_changed(self):
         if self.random:

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
+        if len(ex.args) == 2:
-        logger.info(
+            found, required = ex.args
-            'Disabled extension %s: %s required, but found %s at %s',
+            logger.info(
-            extension.ext_name, required, found, found.location)
+                '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:

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])

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:

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 [])

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
 
 

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

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:
+        with open(file_path, 'wb') as fh:
-            if content:
+            if content is not None:
                 fh.write(content)
     return file_path
 

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):

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):

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'),

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')

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},
+            {'__model__': 'Artist', 'uri': '', 'name': ''},
-            Artist(uri='', name=None).serialize())
+            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', "
+            "Album(artists=[Artist(name=u'foo')], name=u'name', uri=u'uri')",
-            "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', "
+            "Track(name=u'name', uri=u'uri')",
-            "performers=[], 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', "
+            "Track(artists=[Artist(name=u'foo')], name=u'name', uri=u'uri')",
-            "performers=[], 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=[], "
+            "TlTrack(tlid=123, track=Track(uri=u'uri'))",
-            "performers=[], 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=[], "
+            "Playlist(name=u'name', tracks=[Track(name=u'foo')], uri=u'uri')",
-            "name=u'foo', performers=[])], 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):

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('0.19.3'), SV('0.19.4'))
-        self.assertLess(SV(__version__), SV('0.19.5'))
+        self.assertLess(SV('0.19.4'), SV(__version__))
+        self.assertLess(SV(__version__), SV('0.19.6'))

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):