Release v0.19.4

docs/changelog.rst

@@ -5,6 +5,38 @@ Changelog
 This changelog is used to track all major changes to Mopidy.
 
 
+v0.19.4 (2014-09-01)
+====================
+
+Bug fix release.
+
+- Configuration: :option:`mopidy --config` now supports directories.
+
+- Logging: Fix that some loggers would be disabled if
+  :confval:`logging/config_file` was set. (Fixes: :issue:`740`)
+
+- Quit process with exit code 1 when stopping because of a backend, frontend,
+  or mixer initialization error.
+
+- Backend API: Update :meth:`mopidy.backend.LibraryProvider.browse` signature
+  and docs to match how the core use the backend's browse method. (Fixes:
+  :issue:`833`)
+
+- Local library API: Add :attr:`mopidy.local.Library.ROOT_DIRECTORY_URI`
+  constant for use by implementors of :method:`mopidy.local.Library.browse`.
+  (Related to: :issue:`833`)
+
+- HTTP frontend: Guard against double close of WebSocket, which causes an
+  :exc:`AttributeError` on Tornado < 3.2.
+
+- MPD frontend: Make the ``list`` command return albums when sending 3
+  arguments. This was incorrectly returning artists after the MPD command
+  changes in 0.19.0. (Fixes: :issue:`817`)
+
+- MPD frontend: Fix a race condition where two threads could try to free the
+  same data simultaneously. (Fixes: :issue:`781`)
+
+
 v0.19.3 (2014-08-03)
 ====================
 

docs/command.rst

@@ -50,11 +50,12 @@ Options
     Save debug log to the file specified in the :confval:`logging/debug_file`
     config value, typically ``./mopidy.log``.
 
-.. cmdoption:: --config <file>
+.. cmdoption:: --config <file|directory>
 
-    Specify config file to use. To use multiple config files, separate them
-    with a colon. The later files override the earlier ones if there's a
-    conflict.
+    Specify config files and directories to use. To use multiple config files
+    or directories, separate them with a colon. The later files override the
+    earlier ones if there's a conflict. When specifying a directory, all files
+    ending in .conf in the directory are used.
 
 .. cmdoption:: --option <option>, -o <option>
 

docs/installation/debian.rst

@@ -14,6 +14,22 @@ instructions should work for you as well. If you're setting up a Raspberry Pi
 from scratch, we have a guide for installing Debian/Raspbian and Mopidy. See
 :ref:`raspberrypi-installation`.
 
+.. note::
+
+   The packages should work with:
+
+   - Debian stable and testing,
+   - Raspbian stable and testing,
+   - Ubuntu 14.04 LTS and later.
+
+   Some of the packages, including the core "mopidy" packages, does *not* work
+   on Ubuntu 12.04 LTS.
+
+   This is just what we currently support, not a promise to continue to
+   support the same in the future. We *will* drop support for older
+   distributions when supporting those stops us from moving forward with the
+   project.
+
 #. Add the archive's GPG key::
 
        wget -q -O - https://apt.mopidy.com/mopidy.gpg | sudo apt-key add -

mopidy/__init__.py

@@ -21,4 +21,4 @@ if (isinstance(pykka.__version__, basestring)
 warnings.filterwarnings('ignore', 'could not open display')
 
 
-__version__ = '0.19.3'
+__version__ = '0.19.4'

mopidy/backend/__init__.py

@@ -81,12 +81,12 @@ class LibraryProvider(object):
     def __init__(self, backend):
         self.backend = backend
 
-    def browse(self, path):
+    def browse(self, uri):
         """
         See :meth:`mopidy.core.LibraryController.browse`.
 
         If you implement this method, make sure to also set
-        :attr:`root_directory_name`.
+        :attr:`root_directory`.
 
         *MAY be implemented by subclass.*
         """

mopidy/commands.py

@@ -265,6 +265,7 @@ class RootCommand(Command):
         backend_classes = args.registry['backend']
         frontend_classes = args.registry['frontend']
 
+        exit_status_code = 0
         try:
             mixer = self.start_mixer(config, mixer_class)
             audio = self.start_audio(config, mixer)
@@ -276,8 +277,11 @@ class RootCommand(Command):
                 exceptions.FrontendError,
                 exceptions.MixerError):
             logger.info('Initialization error. Exiting...')
+            exit_status_code = 1
         except KeyboardInterrupt:
             logger.info('Interrupted. Exiting...')
+        except Exception:
+            logger.exception('Uncaught exception')
         finally:
             loop.quit()
             self.stop_frontends(frontend_classes)
@@ -286,6 +290,7 @@ class RootCommand(Command):
             self.stop_audio()
             self.stop_mixer(mixer_class)
             process.stop_remaining_actors()
+            return exit_status_code
 
     def get_mixer_class(self, config, mixer_classes):
         logger.debug(

mopidy/config/__init__.py

@@ -116,21 +116,14 @@ def _load(files, defaults, overrides):
         parser.readfp(io.BytesIO(default))
 
     # Load config from a series of config files
-    for filename in files:
-        try:
-            with io.open(filename, 'rb') as filehandle:
-                parser.readfp(filehandle)
-        except configparser.MissingSectionHeaderError as e:
-            logger.warning('%s does not have a config section, not loaded.',
-                           filename)
-        except configparser.ParsingError as e:
-            linenos = ', '.join(str(lineno) for lineno, line in e.errors)
-            logger.warning(
-                '%s has errors, line %s has been ignored.', filename, linenos)
-        except IOError:
-            # TODO: if this is the initial load of logging config we might not
-            # have a logger at this point, we might want to handle this better.
-            logger.debug('Config file %s not found; skipping', filename)
+    for name in files:
+        if os.path.isdir(name):
+            for filename in os.listdir(name):
+                filename = os.path.join(name, filename)
+                if os.path.isfile(filename) and filename.endswith('.conf'):
+                    _load_file(parser, filename)
+        else:
+            _load_file(parser, name)
 
     # If there have been parse errors there is a python bug that causes the
     # values to be lists, this little trick coerces these into strings.
@@ -146,6 +139,23 @@ def _load(files, defaults, overrides):
     return raw_config
 
 
+def _load_file(parser, filename):
+    try:
+        with io.open(filename, 'rb') as filehandle:
+            parser.readfp(filehandle)
+    except configparser.MissingSectionHeaderError as e:
+        logger.warning('%s does not have a config section, not loaded.',
+                       filename)
+    except configparser.ParsingError as e:
+        linenos = ', '.join(str(lineno) for lineno, line in e.errors)
+        logger.warning(
+            '%s has errors, line %s has been ignored.', filename, linenos)
+    except IOError:
+        # TODO: if this is the initial load of logging config we might not
+        # have a logger at this point, we might want to handle this better.
+        logger.debug('Config file %s not found; skipping', filename)
+
+
 def _validate(raw_config, schemas):
     # Get validated config
     config = {}

mopidy/http/handlers.py

@@ -110,7 +110,10 @@ class WebSocketHandler(tornado.websocket.WebSocketHandler):
                     self.request.remote_ip, response)
         except Exception as e:
             logger.error('WebSocket request error: %s', e)
-            self.close()
+            if self.ws_connection:
+                # Tornado 3.2+ checks if self.ws_connection is None before
+                # using it, but not older versions.
+                self.close()
 
     def check_origin(self, origin):
         # Allow cross-origin WebSocket connections, like Tornado before 4.0

mopidy/local/__init__.py

@@ -58,17 +58,28 @@ class Library(object):
     :param config: Config dictionary
     """
 
+    ROOT_DIRECTORY_URI = 'local:directory'
+    """
+    URI of the local backend's root directory.
+
+    This constant should be used by libraries implementing the
+    :meth:`Library.browse` method.
+    """
+
     #: Name of the local library implementation, must be overriden.
     name = None
 
     def __init__(self, config):
         self._config = config
 
-    def browse(self, path):
+    def browse(self, uri):
         """
-        Browse directories and tracks at the given path.
+        Browse directories and tracks at the given URI.
 
-        :param string path: path to browse or None for root.
+        The URI for the root directory is a constant available at
+        :attr:`Library.ROOT_DIRECTORY_URI`.
+
+        :param string path: URI to browse.
         :rtype: List of :class:`~mopidy.models.Ref` tracks and directories.
         """
         raise NotImplementedError

mopidy/local/json.py

@@ -61,8 +61,8 @@ class _BrowseCache(object):
     splitpath_re = re.compile(r'([^/]+)')
 
     def __init__(self, uris):
-        # TODO: local.ROOT_DIRECTORY_URI
-        self._cache = {'local:directory': collections.OrderedDict()}
+        self._cache = {
+            local.Library.ROOT_DIRECTORY_URI: collections.OrderedDict()}
 
         for track_uri in uris:
             path = translator.local_track_uri_to_path(track_uri, b'/')
@@ -97,10 +97,11 @@ class _BrowseCache(object):
             else:
                 # Loop completed, so final child needs to be added to root.
                 if child:
-                    self._cache['local:directory'][child.uri] = child
+                    self._cache[
+                        local.Library.ROOT_DIRECTORY_URI][child.uri] = child
                 # If no parent was set we belong in the root.
                 if not parent_uri:
-                    parent_uri = 'local:directory'
+                    parent_uri = local.Library.ROOT_DIRECTORY_URI
 
             self._cache[parent_uri][track_uri] = track_ref
 

mopidy/local/library.py

@@ -2,7 +2,7 @@ from __future__ import unicode_literals
 
 import logging
 
-from mopidy import backend, models
+from mopidy import backend, local, models
 
 logger = logging.getLogger(__name__)
 
@@ -10,18 +10,18 @@ logger = logging.getLogger(__name__)
 class LocalLibraryProvider(backend.LibraryProvider):
     """Proxy library that delegates work to our active local library."""
 
-    root_directory = models.Ref.directory(uri=b'local:directory',
-                                          name='Local media')
+    root_directory = models.Ref.directory(
+        uri=local.Library.ROOT_DIRECTORY_URI, name='Local media')
 
     def __init__(self, backend, library):
         super(LocalLibraryProvider, self).__init__(backend)
         self._library = library
         self.refresh()
 
-    def browse(self, path):
+    def browse(self, uri):
         if not self._library:
             return []
-        return self._library.browse(path)
+        return self._library.browse(uri)
 
     def refresh(self, uri=None):
         if not self._library:

mopidy/mpd/protocol/music_db.py

@@ -257,7 +257,7 @@ def list_(context, *args):
     if len(parameters) == 1:
         if field != 'album':
             raise exceptions.MpdArgError('should be "Album" for 3 arguments')
-        return _list_artist(context, {'artist': parameters})
+        return _list_album(context, {'artist': parameters})
 
     try:
         query = _query_from_mpd_search_parameters(parameters, _LIST_MAPPING)

mopidy/utils/log.py

@@ -47,7 +47,8 @@ def setup_logging(config, verbosity_level, save_debug_log):
     if config['logging']['config_file']:
         # Logging config from file must be read before other handlers are
         # added. If not, the other handlers will have no effect.
-        logging.config.fileConfig(config['logging']['config_file'])
+        logging.config.fileConfig(config['logging']['config_file'],
+                                  disable_existing_loggers=False)
 
     setup_console_logging(config, verbosity_level)
     if save_debug_log:

mopidy/utils/network.py

@@ -268,8 +268,8 @@ class Connection(object):
             return True
 
         if not data:
-            self.actor_ref.tell({'close': True})
             self.disable_recv()
+            self.actor_ref.tell({'close': True})
             return True
 
         try:

tests/config/test_config.py

@@ -60,6 +60,18 @@ class LoadConfigTest(unittest.TestCase):
         result = config._load([file1, file2], [], [])
         self.assertEqual(expected, result)
 
+    def test_load_directory(self):
+        directory = path_to_data_dir('conf1.d')
+        expected = {'foo': {'bar': 'baz'}, 'foo2': {'bar': 'baz'}}
+        result = config._load([directory], [], [])
+        self.assertEqual(expected, result)
+
+    def test_load_directory_only_conf_files(self):
+        directory = path_to_data_dir('conf2.d')
+        expected = {'foo': {'bar': 'baz'}}
+        result = config._load([directory], [], [])
+        self.assertEqual(expected, result)
+
     def test_load_file_with_utf8(self):
         expected = {'foo': {'bar': 'æøå'.encode('utf-8')}}
         result = config._load([path_to_data_dir('file3.conf')], [], [])

tests/data/conf1.d/file1.conf

@@ -0,0 +1,2 @@
+[foo]
+bar = baz

tests/data/conf1.d/file2.conf

@@ -0,0 +1,2 @@
+[foo2]
+bar = baz

tests/data/conf2.d/file1.conf

@@ -0,0 +1,2 @@
+[foo]
+bar = baz

tests/data/conf2.d/file2.conf.disabled

@@ -0,0 +1,2 @@
+[foo2]
+bar = baz

tests/mpd/protocol/test_music_db.py

@@ -880,7 +880,11 @@ class MusicDatabaseListTest(protocol.BaseTestCase):
         self.assertInResponse('OK')
 
     def test_list_album_with_artist_name(self):
+        self.backend.library.dummy_find_exact_result = SearchResult(
+            tracks=[Track(album=Album(name='foo'))])
+
         self.sendRequest('list "album" "anartist"')
+        self.assertInResponse('Album: foo')
         self.assertInResponse('OK')
 
     def test_list_album_with_artist_name_without_filter_value(self):

tests/test_version.py

@@ -49,5 +49,6 @@ class VersionTest(unittest.TestCase):
         self.assertLess(SV('0.18.3'), SV('0.19.0'))
         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(__version__))
-        self.assertLess(SV(__version__), SV('0.19.4'))
+        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'))

tests/utils/network/test_connection.py

@@ -7,7 +7,7 @@ import unittest
 
 import gobject
 
-from mock import Mock, patch, sentinel
+from mock import Mock, call, patch, sentinel
 
 import pykka
 
@@ -418,8 +418,11 @@ class ConnectionTest(unittest.TestCase):
 
         self.assertTrue(network.Connection.recv_callback(
             self.mock, sentinel.fd, gobject.IO_IN))
-        self.mock.actor_ref.tell.assert_called_once_with({'close': True})
-        self.mock.disable_recv.assert_called_once_with()
+        self.assertEqual(self.mock.mock_calls, [
+            call.sock.recv(any_int),
+            call.disable_recv(),
+            call.actor_ref.tell({'close': True}),
+        ])
 
     def test_recv_callback_recoverable_error(self):
         self.mock.sock = Mock(spec=socket.SocketType)