From fdb0b4a12b23c7abffbbc2d871d3356a5349d178 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 22 Jul 2015 19:39:39 +0200 Subject: [PATCH 01/94] file: Add tests --- tests/file/__init__.py | 0 tests/file/conftest.py | 18 ++++++++++++++++++ tests/file/test_browse.py | 1 + tests/file/test_lookup.py | 1 + 4 files changed, 20 insertions(+) create mode 100644 tests/file/__init__.py create mode 100644 tests/file/conftest.py create mode 100644 tests/file/test_browse.py create mode 100644 tests/file/test_lookup.py diff --git a/tests/file/__init__.py b/tests/file/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/file/conftest.py b/tests/file/conftest.py new file mode 100644 index 00000000..c93a71f2 --- /dev/null +++ b/tests/file/conftest.py @@ -0,0 +1,18 @@ +from __future__ import unicode_literals + +import pytest + +from mopidy.file import library + + +@pytest.fixture +def file_config(): + return { + 'file': { + } + } + + +@pytest.fixture +def file_library(file_config): + return library.FileLibraryProvider(backend=None, config=file_config) diff --git a/tests/file/test_browse.py b/tests/file/test_browse.py new file mode 100644 index 00000000..baffc488 --- /dev/null +++ b/tests/file/test_browse.py @@ -0,0 +1 @@ +from __future__ import unicode_literals diff --git a/tests/file/test_lookup.py b/tests/file/test_lookup.py new file mode 100644 index 00000000..baffc488 --- /dev/null +++ b/tests/file/test_lookup.py @@ -0,0 +1 @@ +from __future__ import unicode_literals From ee0f2f2a9429fd3d07f555651f65f5d7fe0fc577 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 22 Jul 2015 19:47:55 +0200 Subject: [PATCH 02/94] docs: Remove extra word --- docs/extensiondev.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/extensiondev.rst b/docs/extensiondev.rst index 340a18da..77ce7cde 100644 --- a/docs/extensiondev.rst +++ b/docs/extensiondev.rst @@ -438,7 +438,7 @@ When writing an extension, you should only use APIs documented at at any time and are not something extensions should use. Mopidy performs type checking to help catch extension bugs. This applies to -both to frontend calls into core and return values from backends. Additionally +both frontend calls into core and return values from backends. Additionally model fields always get validated to further guard against bad data. Logging in extensions From 0d3563441f5d92731468f87818acbda7f3df3df6 Mon Sep 17 00:00:00 2001 From: Stein Karlsen Date: Sat, 25 Jul 2015 03:43:35 +0200 Subject: [PATCH 03/94] Fixed changed data-structure passed when creating config file, issue 1228 --- mopidy/__main__.py | 4 ++-- mopidy/config/__init__.py | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 245a03ce..ee359268 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -166,7 +166,7 @@ def main(): raise -def create_file_structures_and_config(args, extensions): +def create_file_structures_and_config(args, extensions_data): path.get_or_create_dir(b'$XDG_DATA_DIR/mopidy') path.get_or_create_dir(b'$XDG_CONFIG_DIR/mopidy') @@ -176,7 +176,7 @@ def create_file_structures_and_config(args, extensions): return try: - default = config_lib.format_initial(extensions) + default = config_lib.format_initial(extensions_data) path.get_or_create_file(config_file, mkdir=False, content=default) logger.info('Initialized %s with default config', config_file) except IOError as error: diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 13a26412..0de264f2 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -87,20 +87,23 @@ def format(config, ext_schemas, comments=None, display=True): return _format(config, comments or {}, schemas, display, False) -def format_initial(extensions): +def format_initial(extensions_data): config_dir = os.path.dirname(__file__) defaults = [read(os.path.join(config_dir, 'default.conf'))] - defaults.extend(e.get_default_config() for e in extensions) + defaults.extend(d.extension.get_default_config() for d in extensions_data) raw_config = _load([], defaults, []) schemas = _schemas[:] - schemas.extend(e.get_config_schema() for e in extensions) + schemas.extend(d.extension.get_config_schema() for d in extensions_data) config, errors = _validate(raw_config, schemas) 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)) + extensions_data = sorted( + extensions_data, key=lambda d: d.extension.dist_name) + for data in extensions_data: + versions.append('%s %s' % ( + data.extension.dist_name, data.extension.version)) header = _INITIAL_HELP.strip() % {'versions': '\n# '.join(versions)} formatted_config = _format( From b9c0d305cdc09117821a3d870ec6d7229ae2aacb Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 25 Jul 2015 11:51:45 +0200 Subject: [PATCH 04/94] docs: Add core/max_tracklist_length config --- docs/config.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/config.rst b/docs/config.rst index 46b15635..3a919b5c 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -57,6 +57,18 @@ Core configuration values Mopidy's core has the following configuration values that you can change. + +Core configuration +------------------ + +.. confval:: core/max_tracklist_length + + Max length of the tracklist. Defaults to 10000. + + The original MPD server only supports 10000 tracks in the tracklist. Some + MPD clients will crash if this limit is exceeded. + + Audio configuration ------------------- From 448e929e75741bdfaa91761053eb47afe15c9117 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 25 Jul 2015 11:59:29 +0200 Subject: [PATCH 05/94] file: Add todos for missing tests --- tests/file/test_browse.py | 2 ++ tests/file/test_lookup.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tests/file/test_browse.py b/tests/file/test_browse.py index baffc488..81021966 100644 --- a/tests/file/test_browse.py +++ b/tests/file/test_browse.py @@ -1 +1,3 @@ from __future__ import unicode_literals + +# TODO Test browse() diff --git a/tests/file/test_lookup.py b/tests/file/test_lookup.py index baffc488..19c07181 100644 --- a/tests/file/test_lookup.py +++ b/tests/file/test_lookup.py @@ -1 +1,3 @@ from __future__ import unicode_literals + +# TODO Test lookup() From 33cef9c56ea857df916c6613823442b8a6319ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dejan=20Proki=C4=87?= Date: Sat, 25 Jul 2015 12:21:57 +0200 Subject: [PATCH 06/94] config: Add config for data/config/cache directories Issue #843 --- docs/config.rst | 12 ++++++++++++ mopidy/config/__init__.py | 3 +++ mopidy/config/default.conf | 3 +++ 3 files changed, 18 insertions(+) diff --git a/docs/config.rst b/docs/config.rst index 3a919b5c..e12c152e 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -68,6 +68,18 @@ Core configuration The original MPD server only supports 10000 tracks in the tracklist. Some MPD clients will crash if this limit is exceeded. +.. confval:: core/cache_dir + + Path to directory for storing cached information. + +.. confval:: core/config_dir + + Path to directory with configuration files. + +.. confval:: core/data_dir + + Path to directory with data files. + Audio configuration ------------------- diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 13a26412..df5b73bc 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -18,6 +18,9 @@ logger = logging.getLogger(__name__) _core_schema = ConfigSchema('core') # MPD supports at most 10k tracks, some clients segfault when this is exceeded. _core_schema['max_tracklist_length'] = Integer(minimum=1, maximum=10000) +_core_schema['cache_dir'] = Path() +_core_schema['config_dir'] = Path() +_core_schema['data_dir'] = Path() _logging_schema = ConfigSchema('logging') _logging_schema['color'] = Boolean() diff --git a/mopidy/config/default.conf b/mopidy/config/default.conf index c214de68..5e749f0c 100644 --- a/mopidy/config/default.conf +++ b/mopidy/config/default.conf @@ -1,5 +1,8 @@ [core] max_tracklist_length = 10000 +cache_dir = $XDG_CACHE_DIR/mopidy +config_dir = $XDG_CONFIG_DIR/mopidy +data_dir = $XDG_DATA_DIR/mopidy [logging] color = true From ad2646f1ab2134b40df5ff87a8ce2034b4cea750 Mon Sep 17 00:00:00 2001 From: EricJahn Date: Sat, 25 Jul 2015 13:15:37 +0200 Subject: [PATCH 07/94] tests: Fix failing encoding test Encoding is different on Mac OS and needs to be checked separately. --- tests/m3u/test_playlists.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index f490887a..edebe65b 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals import os +import platform import shutil import tempfile import unittest @@ -156,8 +157,11 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.core.playlists.refresh() self.assertEqual(len(self.core.playlists.as_list()), 1) - result = self.core.playlists.lookup(uri) - self.assertEqual('\ufffd\ufffd\ufffd', result.name) + result = self.core.playlists.as_list() + if platform.system() == 'Darwin': + self.assertEqual('%F8%E6%E5', result[0].name) + else: + self.assertEqual('\ufffd\ufffd\ufffd', result[0].name) @unittest.SkipTest def test_playlists_dir_is_created(self): From 0ae6239988bef8fbe2250d9a68c0cf3aa1ee4c52 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 25 Jul 2015 11:48:54 +0200 Subject: [PATCH 08/94] config: Add regression test for format_initial() --- tests/config/test_config.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/config/test_config.py b/tests/config/test_config.py index 139f3a69..fa2285b8 100644 --- a/tests/config/test_config.py +++ b/tests/config/test_config.py @@ -6,7 +6,7 @@ import unittest import mock -from mopidy import config +from mopidy import config, ext from tests import path_to_data_dir @@ -292,3 +292,23 @@ class PostProcessorTest(unittest.TestCase): def test_conversion(self): result = config._postprocess(PROCESSED_CONFIG) self.assertEqual(result, INPUT_CONFIG) + + +def test_format_initial(): + extension = ext.Extension() + extension.ext_name = 'foo' + extension.get_default_config = lambda: None + extensions_data = [ + ext.ExtensionData( + extension=extension, + entry_point=None, + config_schema=None, + config_defaults=None, + command=None, + ), + ] + + result = config.format_initial(extensions_data) + + assert '# For further information' in result + assert '[foo]\n' in result From dd92a59e4c91c7b46fbd8933c7e1cff06b15eecf Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 25 Jul 2015 14:29:22 +0200 Subject: [PATCH 09/94] docs: Update .mailmap and authors --- .mailmap | 2 ++ AUTHORS | 3 +++ 2 files changed, 5 insertions(+) diff --git a/.mailmap b/.mailmap index 718d8f4b..0682f673 100644 --- a/.mailmap +++ b/.mailmap @@ -24,4 +24,6 @@ Christopher Schirner John Cass Ronald Zielaznicki +Kyle Heyne Tom Roth +Eric Jahn diff --git a/AUTHORS b/AUTHORS index 258967c3..c2baca6c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -57,5 +57,8 @@ - Camilo Nova - Dražen Lučanin - Naglis Jonaitis +- Kyle Heyne - Tom Roth - Mark Greenwood +- Stein Karlsen +- Eric Jahn From 0bf67e296f2e6be27448c49665db2a16dc088340 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 23 Jul 2015 19:14:08 +0200 Subject: [PATCH 10/94] core: Log full backend exception --- mopidy/core/playback.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index f374127a..9a11066b 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -348,8 +348,11 @@ class PlaybackController(object): backend.playback.change_track(tl_track.track).get() and backend.playback.play().get()) except TypeError: - logger.error('%s needs to be updated to work with this ' - 'version of Mopidy.', backend) + logger.error( + '%s needs to be updated to work with this ' + 'version of Mopidy.', + backend.actor_ref.actor_class.__name__) + logger.debug('Backend exception', exc_info=True) if success: self.core.tracklist._mark_playing(tl_track) From 1a2cb8f6b55305f5749464ce81c4c2b1b76362e7 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 23 Jul 2015 13:12:06 +0200 Subject: [PATCH 11/94] audio: Move IcySrc out of playlist module --- mopidy/audio/actor.py | 3 +- mopidy/audio/icy.py | 63 +++++++++++++++++++++++++++++++++++++++ mopidy/audio/playlists.py | 52 -------------------------------- 3 files changed, 65 insertions(+), 53 deletions(-) create mode 100644 mopidy/audio/icy.py diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 72750bdf..5946bd73 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -13,7 +13,7 @@ import gst.pbutils # noqa import pykka from mopidy import exceptions -from mopidy.audio import playlists, utils +from mopidy.audio import icy, playlists, utils from mopidy.audio.constants import PlaybackState from mopidy.audio.listener import AudioListener from mopidy.internal import deprecation, process @@ -26,6 +26,7 @@ logger = logging.getLogger(__name__) # set_state on a pipeline. gst_logger = logging.getLogger('mopidy.audio.gst') +icy.register() playlists.register_typefinders() playlists.register_elements() diff --git a/mopidy/audio/icy.py b/mopidy/audio/icy.py new file mode 100644 index 00000000..dd59baae --- /dev/null +++ b/mopidy/audio/icy.py @@ -0,0 +1,63 @@ +from __future__ import absolute_import, unicode_literals + +import gobject + +import pygst +pygst.require('0.10') +import gst # noqa + + +class IcySrc(gst.Bin, gst.URIHandler): + __gstdetails__ = ('IcySrc', + 'Src', + 'HTTP src wrapper for icy:// support.', + 'Mopidy') + + srcpad_template = gst.PadTemplate( + 'src', gst.PAD_SRC, gst.PAD_ALWAYS, + gst.caps_new_any()) + + __gsttemplates__ = (srcpad_template,) + + def __init__(self): + super(IcySrc, self).__init__() + self._httpsrc = gst.element_make_from_uri(gst.URI_SRC, 'http://') + try: + self._httpsrc.set_property('iradio-mode', True) + except TypeError: + pass + self.add(self._httpsrc) + + self._srcpad = gst.GhostPad('src', self._httpsrc.get_pad('src')) + self.add_pad(self._srcpad) + + @classmethod + def do_get_type_full(cls): + return gst.URI_SRC + + @classmethod + def do_get_protocols_full(cls): + return [b'icy', b'icyx'] + + def do_set_uri(self, uri): + if uri.startswith('icy://'): + return self._httpsrc.set_uri(b'http://' + uri[len('icy://'):]) + elif uri.startswith('icyx://'): + return self._httpsrc.set_uri(b'https://' + uri[len('icyx://'):]) + else: + return False + + def do_get_uri(self): + uri = self._httpsrc.get_uri() + if uri.startswith('http://'): + return b'icy://' + uri[len('http://'):] + else: + return b'icyx://' + uri[len('https://'):] + + +def register(): + # Only register icy if gst install can't handle it on it's own. + if not gst.element_make_from_uri(gst.URI_SRC, 'icy://'): + gobject.type_register(IcySrc) + gst.element_register( + IcySrc, IcySrc.__name__.lower(), gst.RANK_MARGINAL) diff --git a/mopidy/audio/playlists.py b/mopidy/audio/playlists.py index 58c7fe24..ddbd9c66 100644 --- a/mopidy/audio/playlists.py +++ b/mopidy/audio/playlists.py @@ -354,54 +354,6 @@ class UriListElement(BasePlaylistElement): return parse_urilist(data) -class IcySrc(gst.Bin, gst.URIHandler): - __gstdetails__ = ('IcySrc', - 'Src', - 'HTTP src wrapper for icy:// support.', - 'Mopidy') - - srcpad_template = gst.PadTemplate( - 'src', gst.PAD_SRC, gst.PAD_ALWAYS, - gst.caps_new_any()) - - __gsttemplates__ = (srcpad_template,) - - def __init__(self): - super(IcySrc, self).__init__() - self._httpsrc = gst.element_make_from_uri(gst.URI_SRC, 'http://') - try: - self._httpsrc.set_property('iradio-mode', True) - except TypeError: - pass - self.add(self._httpsrc) - - self._srcpad = gst.GhostPad('src', self._httpsrc.get_pad('src')) - self.add_pad(self._srcpad) - - @classmethod - def do_get_type_full(cls): - return gst.URI_SRC - - @classmethod - def do_get_protocols_full(cls): - return [b'icy', b'icyx'] - - def do_set_uri(self, uri): - if uri.startswith('icy://'): - return self._httpsrc.set_uri(b'http://' + uri[len('icy://'):]) - elif uri.startswith('icyx://'): - return self._httpsrc.set_uri(b'https://' + uri[len('icyx://'):]) - else: - return False - - def do_get_uri(self): - uri = self._httpsrc.get_uri() - if uri.startswith('http://'): - return b'icy://' + uri[len('http://'):] - else: - return b'icyx://' + uri[len('https://'):] - - def register_element(element_class): gobject.type_register(element_class) gst.element_register( @@ -414,7 +366,3 @@ def register_elements(): register_element(XspfDecoder) register_element(AsxDecoder) register_element(UriListElement) - - # Only register icy if gst install can't handle it on it's own. - if not gst.element_make_from_uri(gst.URI_SRC, 'icy://'): - register_element(IcySrc) From c0e9593a0bdf943782e2808bf497c5b1bdce72b9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 23 Jul 2015 13:18:12 +0200 Subject: [PATCH 12/94] audio: Remove playlist elements --- mopidy/audio/actor.py | 4 +- mopidy/audio/playlists.py | 261 -------------------------------------- 2 files changed, 1 insertion(+), 264 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 5946bd73..60e88a9d 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -13,7 +13,7 @@ import gst.pbutils # noqa import pykka from mopidy import exceptions -from mopidy.audio import icy, playlists, utils +from mopidy.audio import icy, utils from mopidy.audio.constants import PlaybackState from mopidy.audio.listener import AudioListener from mopidy.internal import deprecation, process @@ -27,8 +27,6 @@ logger = logging.getLogger(__name__) gst_logger = logging.getLogger('mopidy.audio.gst') icy.register() -playlists.register_typefinders() -playlists.register_elements() _GST_STATE_MAPPING = { gst.STATE_PLAYING: PlaybackState.PLAYING, diff --git a/mopidy/audio/playlists.py b/mopidy/audio/playlists.py index ddbd9c66..34e3ed7a 100644 --- a/mopidy/audio/playlists.py +++ b/mopidy/audio/playlists.py @@ -2,8 +2,6 @@ from __future__ import absolute_import, unicode_literals import io -import gobject - import pygst pygst.require('0.10') import gst # noqa @@ -107,262 +105,3 @@ def parse_asx(data): for entry in element.findall('entry[@href]'): yield entry.get('href', '').strip() - - -def parse_urilist(data): - for line in data.readlines(): - if not line.startswith('#') and gst.uri_is_valid(line.strip()): - yield line - - -def playlist_typefinder(typefind, func, caps): - if func(typefind): - typefind.suggest(gst.TYPE_FIND_MAXIMUM, caps) - - -def register_typefind(mimetype, func, extensions): - caps = gst.caps_from_string(mimetype) - gst.type_find_register(mimetype, gst.RANK_PRIMARY, playlist_typefinder, - extensions, caps, func, caps) - - -def register_typefinders(): - register_typefind('audio/x-mpegurl', detect_m3u_header, [b'm3u', b'm3u8']) - register_typefind('audio/x-scpls', detect_pls_header, [b'pls']) - register_typefind('application/xspf+xml', detect_xspf_header, [b'xspf']) - # NOTE: seems we can't use video/x-ms-asf which is the correct mime for asx - # as it is shared with asf for streaming videos :/ - register_typefind('audio/x-ms-asx', detect_asx_header, [b'asx']) - - -class BasePlaylistElement(gst.Bin): - - """Base class for creating GStreamer elements for playlist support. - - This element performs the following steps: - - 1. Initializes src and sink pads for the element. - 2. Collects data from the sink until EOS is reached. - 3. Passes the collected data to :meth:`convert` to get a list of URIs. - 4. Passes the list of URIs to :meth:`handle`, default handling is to pass - the URIs to the src element as a uri-list. - 5. If handle returned true, the EOS consumed and nothing more happens, if - it is not consumed it flows on to the next element downstream, which is - likely our uri-list consumer which needs the EOS to know we are done - sending URIs. - """ - - sinkpad_template = None - """GStreamer pad template to use for sink, must be overriden.""" - - srcpad_template = None - """GStreamer pad template to use for src, must be overriden.""" - - ghost_srcpad = False - """Indicates if src pad should be ghosted or not.""" - - def __init__(self): - """Sets up src and sink pads plus behaviour.""" - super(BasePlaylistElement, self).__init__() - self._data = io.BytesIO() - self._done = False - - self.sinkpad = gst.Pad(self.sinkpad_template) - self.sinkpad.set_chain_function(self._chain) - self.sinkpad.set_event_function(self._event) - self.add_pad(self.sinkpad) - - if self.ghost_srcpad: - self.srcpad = gst.ghost_pad_new_notarget('src', gst.PAD_SRC) - else: - self.srcpad = gst.Pad(self.srcpad_template) - self.add_pad(self.srcpad) - - def convert(self, data): - """Convert the data we have colleted to URIs. - - :param data: collected data buffer - :type data: :class:`io.BytesIO` - :returns: iterable or generator of URIs - """ - raise NotImplementedError - - def handle(self, uris): - """Do something useful with the URIs. - - :param uris: list of URIs - :type uris: :type:`list` - :returns: boolean indicating if EOS should be consumed - """ - # TODO: handle unicode uris which we can get out of elementtree - self.srcpad.push(gst.Buffer('\n'.join(uris))) - return False - - def _chain(self, pad, buf): - if not self._done: - self._data.write(buf.data) - return gst.FLOW_OK - return gst.FLOW_EOS - - def _event(self, pad, event): - if event.type == gst.EVENT_NEWSEGMENT: - return True - - if event.type == gst.EVENT_EOS: - self._done = True - self._data.seek(0) - if self.handle(list(self.convert(self._data))): - return True - - # Ensure we handle remaining events in a sane way. - return pad.event_default(event) - - -class M3uDecoder(BasePlaylistElement): - __gstdetails__ = ('M3U Decoder', - 'Decoder', - 'Convert .m3u to text/uri-list', - 'Mopidy') - - sinkpad_template = gst.PadTemplate( - 'sink', gst.PAD_SINK, gst.PAD_ALWAYS, - gst.caps_from_string('audio/x-mpegurl')) - - srcpad_template = gst.PadTemplate( - 'src', gst.PAD_SRC, gst.PAD_ALWAYS, - gst.caps_from_string('text/uri-list')) - - __gsttemplates__ = (sinkpad_template, srcpad_template) - - def convert(self, data): - return parse_m3u(data) - - -class PlsDecoder(BasePlaylistElement): - __gstdetails__ = ('PLS Decoder', - 'Decoder', - 'Convert .pls to text/uri-list', - 'Mopidy') - - sinkpad_template = gst.PadTemplate( - 'sink', gst.PAD_SINK, gst.PAD_ALWAYS, - gst.caps_from_string('audio/x-scpls')) - - srcpad_template = gst.PadTemplate( - 'src', gst.PAD_SRC, gst.PAD_ALWAYS, - gst.caps_from_string('text/uri-list')) - - __gsttemplates__ = (sinkpad_template, srcpad_template) - - def convert(self, data): - return parse_pls(data) - - -class XspfDecoder(BasePlaylistElement): - __gstdetails__ = ('XSPF Decoder', - 'Decoder', - 'Convert .pls to text/uri-list', - 'Mopidy') - - sinkpad_template = gst.PadTemplate( - 'sink', gst.PAD_SINK, gst.PAD_ALWAYS, - gst.caps_from_string('application/xspf+xml')) - - srcpad_template = gst.PadTemplate( - 'src', gst.PAD_SRC, gst.PAD_ALWAYS, - gst.caps_from_string('text/uri-list')) - - __gsttemplates__ = (sinkpad_template, srcpad_template) - - def convert(self, data): - return parse_xspf(data) - - -class AsxDecoder(BasePlaylistElement): - __gstdetails__ = ('ASX Decoder', - 'Decoder', - 'Convert .asx to text/uri-list', - 'Mopidy') - - sinkpad_template = gst.PadTemplate( - 'sink', gst.PAD_SINK, gst.PAD_ALWAYS, - gst.caps_from_string('audio/x-ms-asx')) - - srcpad_template = gst.PadTemplate( - 'src', gst.PAD_SRC, gst.PAD_ALWAYS, - gst.caps_from_string('text/uri-list')) - - __gsttemplates__ = (sinkpad_template, srcpad_template) - - def convert(self, data): - return parse_asx(data) - - -class UriListElement(BasePlaylistElement): - __gstdetails__ = ('URIListDemuxer', - 'Demuxer', - 'Convert a text/uri-list to a stream', - 'Mopidy') - - sinkpad_template = gst.PadTemplate( - 'sink', gst.PAD_SINK, gst.PAD_ALWAYS, - gst.caps_from_string('text/uri-list')) - - srcpad_template = gst.PadTemplate( - 'src', gst.PAD_SRC, gst.PAD_ALWAYS, - gst.caps_new_any()) - - ghost_srcpad = True # We need to hook this up to our internal decodebin - - __gsttemplates__ = (sinkpad_template, srcpad_template) - - def __init__(self): - super(UriListElement, self).__init__() - self.uridecodebin = gst.element_factory_make('uridecodebin') - self.uridecodebin.connect('pad-added', self.pad_added) - # Limit to anycaps so we get a single stream out, letting other - # elements downstream figure out actual muxing - self.uridecodebin.set_property('caps', gst.caps_new_any()) - - def pad_added(self, src, pad): - self.srcpad.set_target(pad) - pad.add_event_probe(self.pad_event) - - def pad_event(self, pad, event): - if event.has_name('urilist-played'): - error = gst.GError(gst.RESOURCE_ERROR, gst.RESOURCE_ERROR_FAILED, - b'Nested playlists not supported.') - message = b'Playlists pointing to other playlists is not supported' - self.post_message(gst.message_new_error(self, error, message)) - return 1 # GST_PAD_PROBE_OK - - def handle(self, uris): - struct = gst.Structure('urilist-played') - event = gst.event_new_custom(gst.EVENT_CUSTOM_UPSTREAM, struct) - self.sinkpad.push_event(event) - - # TODO: hookup about to finish and errors to rest of URIs so we - # round robin, only giving up once all have been tried. - # TODO: uris could be empty. - self.add(self.uridecodebin) - self.uridecodebin.set_state(gst.STATE_READY) - self.uridecodebin.set_property('uri', uris[0]) - self.uridecodebin.sync_state_with_parent() - return True # Make sure we consume the EOS that triggered us. - - def convert(self, data): - return parse_urilist(data) - - -def register_element(element_class): - gobject.type_register(element_class) - gst.element_register( - element_class, element_class.__name__.lower(), gst.RANK_MARGINAL) - - -def register_elements(): - register_element(M3uDecoder) - register_element(PlsDecoder) - register_element(XspfDecoder) - register_element(AsxDecoder) - register_element(UriListElement) From a83d9836f9c00c4d95a8d51dfa7f4e1ab23338ce Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 25 Jul 2015 14:46:48 +0200 Subject: [PATCH 13/94] core: Update test mock --- tests/core/test_playback.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 0e51c4db..5a8c9649 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -724,6 +724,7 @@ class CorePlaybackWithOldBackendTest(unittest.TestCase): } b = mock.Mock() + b.actor_ref.actor_class.__name__ = 'DummyBackend' b.uri_schemes.get.return_value = ['dummy1'] b.playback = mock.Mock(spec=backend.PlaybackProvider) b.playback.play.side_effect = TypeError From bdcfab09f18ca1f7c24f9d5eef7d8149e984650e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 23 Jul 2015 13:19:10 +0200 Subject: [PATCH 14/94] playlists: Move detecters and parsers out of audio --- mopidy/{audio => internal}/playlists.py | 0 tests/{audio => internal}/test_playlists.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename mopidy/{audio => internal}/playlists.py (100%) rename tests/{audio => internal}/test_playlists.py (98%) diff --git a/mopidy/audio/playlists.py b/mopidy/internal/playlists.py similarity index 100% rename from mopidy/audio/playlists.py rename to mopidy/internal/playlists.py diff --git a/tests/audio/test_playlists.py b/tests/internal/test_playlists.py similarity index 98% rename from tests/audio/test_playlists.py rename to tests/internal/test_playlists.py index 769e1592..c5499239 100644 --- a/tests/audio/test_playlists.py +++ b/tests/internal/test_playlists.py @@ -5,7 +5,7 @@ from __future__ import absolute_import, unicode_literals import io import unittest -from mopidy.audio import playlists +from mopidy.internal import playlists BAD = b'foobarbaz' From caff7210558ebb6ed827708aaed6096f12e5d9a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dejan=20Proki=C4=87?= Date: Sat, 25 Jul 2015 15:17:51 +0200 Subject: [PATCH 15/94] config: Reorder core config options As suggested by jodal on pull request #1232 Issue #843 --- docs/config.rst | 14 +++++++------- mopidy/config/__init__.py | 4 ++-- mopidy/config/default.conf | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/config.rst b/docs/config.rst index e12c152e..b6be6cfa 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -61,13 +61,6 @@ Mopidy's core has the following configuration values that you can change. Core configuration ------------------ -.. confval:: core/max_tracklist_length - - Max length of the tracklist. Defaults to 10000. - - The original MPD server only supports 10000 tracks in the tracklist. Some - MPD clients will crash if this limit is exceeded. - .. confval:: core/cache_dir Path to directory for storing cached information. @@ -80,6 +73,13 @@ Core configuration Path to directory with data files. +.. confval:: core/max_tracklist_length + + Max length of the tracklist. Defaults to 10000. + + The original MPD server only supports 10000 tracks in the tracklist. Some + MPD clients will crash if this limit is exceeded. + Audio configuration ------------------- diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index df5b73bc..561d202a 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -16,11 +16,11 @@ from mopidy.internal import path, versioning logger = logging.getLogger(__name__) _core_schema = ConfigSchema('core') -# MPD supports at most 10k tracks, some clients segfault when this is exceeded. -_core_schema['max_tracklist_length'] = Integer(minimum=1, maximum=10000) _core_schema['cache_dir'] = Path() _core_schema['config_dir'] = Path() _core_schema['data_dir'] = Path() +# MPD supports at most 10k tracks, some clients segfault when this is exceeded. +_core_schema['max_tracklist_length'] = Integer(minimum=1, maximum=10000) _logging_schema = ConfigSchema('logging') _logging_schema['color'] = Boolean() diff --git a/mopidy/config/default.conf b/mopidy/config/default.conf index 5e749f0c..675381d9 100644 --- a/mopidy/config/default.conf +++ b/mopidy/config/default.conf @@ -1,8 +1,8 @@ [core] -max_tracklist_length = 10000 cache_dir = $XDG_CACHE_DIR/mopidy config_dir = $XDG_CONFIG_DIR/mopidy data_dir = $XDG_DATA_DIR/mopidy +max_tracklist_length = 10000 [logging] color = true From 1e716c71395d2df5e34fe403115d0dab630f36cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dejan=20Proki=C4=87?= Date: Sat, 25 Jul 2015 16:04:39 +0200 Subject: [PATCH 16/94] tests: Add tests for defaults in core schema config Tests check if default core schema has cache_dir, config_dir, data_dir and max_tracklist_length and if they have proper type --- tests/config/test_defaults.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/config/test_defaults.py diff --git a/tests/config/test_defaults.py b/tests/config/test_defaults.py new file mode 100644 index 00000000..0cf78f6f --- /dev/null +++ b/tests/config/test_defaults.py @@ -0,0 +1,26 @@ +from __future__ import absolute_import, unicode_literals + +from mopidy import config + + +def test_core_schema_has_cache_dir(): + assert 'cache_dir' in config._core_schema + assert isinstance(config._core_schema['cache_dir'], config.Path) + + +def test_core_schema_has_config_dir(): + assert 'config_dir' in config._core_schema + assert isinstance(config._core_schema['config_dir'], config.Path) + + +def test_core_schema_has_data_dir(): + assert 'data_dir' in config._core_schema + assert isinstance(config._core_schema['data_dir'], config.Path) + + +def test_core_schema_has_max_tracklist_length(): + assert 'max_tracklist_length' in config._core_schema + max_tracklist_length_schema = config._core_schema['max_tracklist_length'] + assert isinstance(max_tracklist_length_schema, config.Integer) + assert max_tracklist_length_schema._minimum == 1 + assert max_tracklist_length_schema._maximum == 10000 From e0dc26f958f763e6881f1e4a2dea5c8e31b347d2 Mon Sep 17 00:00:00 2001 From: EricJahn Date: Sat, 25 Jul 2015 16:56:34 +0200 Subject: [PATCH 17/94] Fix #1045 Filter all empty tagtypes in MPD protocol handlers. In addition return the tagtype list from the mpd protocol command. --- mopidy/mpd/protocol/reflection.py | 5 ++++- mopidy/mpd/protocol/tagtype_list.py | 22 ++++++++++++++++++ mopidy/mpd/translator.py | 25 +++++++++++++++++++-- tests/mpd/protocol/test_reflection.py | 17 ++++++++++++++ tests/mpd/protocol/test_status.py | 8 +++---- tests/mpd/protocol/test_stored_playlists.py | 6 ++--- tests/mpd/test_translator.py | 22 +++++++++--------- 7 files changed, 84 insertions(+), 21 deletions(-) create mode 100644 mopidy/mpd/protocol/tagtype_list.py diff --git a/mopidy/mpd/protocol/reflection.py b/mopidy/mpd/protocol/reflection.py index a3608a96..2f96be48 100644 --- a/mopidy/mpd/protocol/reflection.py +++ b/mopidy/mpd/protocol/reflection.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals from mopidy.mpd import exceptions, protocol +from mopidy.mpd.protocol import tagtype_list @protocol.commands.add('config', list_command=False) @@ -93,7 +94,9 @@ def tagtypes(context): Shows a list of available song metadata. """ - pass # TODO + return [ + ('tagtype', tagtype) for tagtype in tagtype_list.TAGTYPE_LIST + ] @protocol.commands.add('urlhandlers') diff --git a/mopidy/mpd/protocol/tagtype_list.py b/mopidy/mpd/protocol/tagtype_list.py new file mode 100644 index 00000000..575b1aaf --- /dev/null +++ b/mopidy/mpd/protocol/tagtype_list.py @@ -0,0 +1,22 @@ +from __future__ import unicode_literals + + +TAGTYPE_LIST = [ + 'Artist', + 'ArtistSort', + 'Album', + 'AlbumArtist', + 'AlbumArtistSort', + 'Title', + 'Track', + 'Name', + 'Genre', + 'Date', + 'Composer', + 'Performer', + 'Disc', + 'MUSICBRAINZ_ARTISTID', + 'MUSICBRAINZ_ALBUMID', + 'MUSICBRAINZ_ALBUMARTISTID', + 'MUSICBRAINZ_TRACKID', +] diff --git a/mopidy/mpd/translator.py b/mopidy/mpd/translator.py index d7ecb0f1..2f3efd1d 100644 --- a/mopidy/mpd/translator.py +++ b/mopidy/mpd/translator.py @@ -4,6 +4,7 @@ import datetime import re from mopidy.models import TlTrack +from mopidy.mpd.protocol import tagtype_list # TODO: special handling of local:// uri scheme normalize_path_re = re.compile(r'[^/]+') @@ -35,8 +36,6 @@ def track_to_mpd_format(track, position=None, stream_title=None): result = [ ('file', track.uri or ''), - # TODO: only show length if not none, see: - # https://github.com/mopidy/mopidy/issues/923#issuecomment-79584110 ('Time', track.length and (track.length // 1000) or 0), ('Artist', concat_multi_values(track.artists, 'name')), ('Album', track.album and track.album.name or ''), @@ -97,9 +96,31 @@ def track_to_mpd_format(track, position=None, stream_title=None): if track.musicbrainz_id is not None: result.append(('MUSICBRAINZ_TRACKID', track.musicbrainz_id)) + + # clean up result + result = [element for element in result if _has_value(*element)] + return result +def _has_value(tagtype, value): + """ + Determine whether to add the tagtype + to the output or not (if they have a default value). + + :param tagtype: the mpd tagtype + :type tagtype: string + :param value: the associated value + :type value: multiple + :rtype: bool + """ + if tagtype in tagtype_list.TAGTYPE_LIST: + if not value: + return False + + return True + + def concat_multi_values(models, attribute): """ Format Mopidy model values for output to MPD client. diff --git a/tests/mpd/protocol/test_reflection.py b/tests/mpd/protocol/test_reflection.py index 4641a8f4..0745479d 100644 --- a/tests/mpd/protocol/test_reflection.py +++ b/tests/mpd/protocol/test_reflection.py @@ -42,6 +42,23 @@ class ReflectionHandlerTest(protocol.BaseTestCase): def test_tagtypes(self): self.send_request('tagtypes') self.assertInResponse('OK') + self.assertInResponse('tagtype: Artist') + self.assertInResponse('tagtype: ArtistSort') + self.assertInResponse('tagtype: Album') + self.assertInResponse('tagtype: AlbumArtist') + self.assertInResponse('tagtype: AlbumArtistSort') + self.assertInResponse('tagtype: Title') + self.assertInResponse('tagtype: Track') + self.assertInResponse('tagtype: Name') + self.assertInResponse('tagtype: Genre') + self.assertInResponse('tagtype: Date') + self.assertInResponse('tagtype: Composer') + self.assertInResponse('tagtype: Performer') + self.assertInResponse('tagtype: Disc') + self.assertInResponse('tagtype: MUSICBRAINZ_ARTISTID') + self.assertInResponse('tagtype: MUSICBRAINZ_ALBUMID') + self.assertInResponse('tagtype: MUSICBRAINZ_ALBUMARTISTID') + self.assertInResponse('tagtype: MUSICBRAINZ_TRACKID') def test_urlhandlers(self): self.send_request('urlhandlers') diff --git a/tests/mpd/protocol/test_status.py b/tests/mpd/protocol/test_status.py index ea4137de..fb448d8d 100644 --- a/tests/mpd/protocol/test_status.py +++ b/tests/mpd/protocol/test_status.py @@ -20,10 +20,10 @@ class StatusHandlerTest(protocol.BaseTestCase): self.send_request('currentsong') self.assertInResponse('file: dummy:/a') self.assertInResponse('Time: 0') - self.assertInResponse('Artist: ') - self.assertInResponse('Title: ') - self.assertInResponse('Album: ') - self.assertInResponse('Track: 0') + self.assertNotInResponse('Artist: ') + self.assertNotInResponse('Title: ') + self.assertNotInResponse('Album: ') + self.assertNotInResponse('Track: 0') self.assertNotInResponse('Date: ') self.assertInResponse('Pos: 0') self.assertInResponse('Id: 0') diff --git a/tests/mpd/protocol/test_stored_playlists.py b/tests/mpd/protocol/test_stored_playlists.py index 90b25a70..2899a126 100644 --- a/tests/mpd/protocol/test_stored_playlists.py +++ b/tests/mpd/protocol/test_stored_playlists.py @@ -45,7 +45,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.send_request('listplaylistinfo "name"') self.assertInResponse('file: dummy:a') - self.assertInResponse('Track: 0') + self.assertNotInResponse('Track: 0') self.assertNotInResponse('Pos: 0') self.assertInResponse('OK') @@ -56,7 +56,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.send_request('listplaylistinfo name') self.assertInResponse('file: dummy:a') - self.assertInResponse('Track: 0') + self.assertNotInResponse('Track: 0') self.assertNotInResponse('Pos: 0') self.assertInResponse('OK') @@ -72,7 +72,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.send_request('listplaylistinfo "a [2]"') self.assertInResponse('file: c') - self.assertInResponse('Track: 0') + self.assertNotInResponse('Track: 0') self.assertNotInResponse('Pos: 0') self.assertInResponse('OK') diff --git a/tests/mpd/test_translator.py b/tests/mpd/test_translator.py index 6a0220a8..57477a51 100644 --- a/tests/mpd/test_translator.py +++ b/tests/mpd/test_translator.py @@ -33,17 +33,17 @@ class TrackMpdFormatTest(unittest.TestCase): path.mtime.undo_fake() def test_track_to_mpd_format_for_empty_track(self): - # TODO: this is likely wrong, see: - # https://github.com/mopidy/mopidy/issues/923#issuecomment-79584110 - result = translator.track_to_mpd_format(Track()) - self.assertIn(('file', ''), result) - self.assertIn(('Time', 0), result) - self.assertIn(('Artist', ''), result) - self.assertIn(('Title', ''), result) - self.assertIn(('Album', ''), result) - self.assertIn(('Track', 0), result) + result = translator.track_to_mpd_format( + Track(uri='a uri', length=137000) + ) + self.assertIn(('file', 'a uri'), result) + self.assertIn(('Time', 137), result) + self.assertNotIn(('Artist', ''), result) + self.assertNotIn(('Title', ''), result) + self.assertNotIn(('Album', ''), result) + self.assertNotIn(('Track', 0), result) self.assertNotIn(('Date', ''), result) - self.assertEqual(len(result), 6) + self.assertEqual(len(result), 2) def test_track_to_mpd_format_with_position(self): result = translator.track_to_mpd_format(Track(), position=1) @@ -137,7 +137,7 @@ class TrackMpdFormatTest(unittest.TestCase): def test_track_to_mpd_format_with_empty_stream_title(self): result = translator.track_to_mpd_format(self.track, stream_title='') self.assertIn(('Name', 'a name'), result) - self.assertIn(('Title', ''), result) + self.assertNotIn(('Title', ''), result) def test_track_to_mpd_format_with_stream_and_no_track_name(self): track = self.track.replace(name=None) From 13133fd9190fd467cfd22806ec48411b98b5c705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dejan=20Proki=C4=87?= Date: Sat, 25 Jul 2015 17:17:13 +0200 Subject: [PATCH 18/94] tests: Make test_ext discoverable Classes in this test didn't have prefix Test --- tests/test_ext.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/test_ext.py b/tests/test_ext.py index 748aebb3..b73e2bae 100644 --- a/tests/test_ext.py +++ b/tests/test_ext.py @@ -11,7 +11,7 @@ from mopidy import config, exceptions, ext from tests import IsA, any_unicode -class TestExtension(ext.Extension): +class DummyExtension(ext.Extension): dist_name = 'Mopidy-Foobar' ext_name = 'foobar' version = '1.2.3' @@ -20,10 +20,10 @@ class TestExtension(ext.Extension): return '[foobar]\nenabled = true' -any_testextension = IsA(TestExtension) +any_testextension = IsA(DummyExtension) -class ExtensionTest(object): +class TestExtension(object): @pytest.fixture def extension(self): @@ -54,7 +54,7 @@ class ExtensionTest(object): extension.setup(None) -class LoadExtensionsTest(object): +class TestLoadExtensions(object): @pytest.yield_fixture def iter_entry_points_mock(self, request): @@ -70,7 +70,7 @@ class LoadExtensionsTest(object): def test_load_extensions(self, iter_entry_points_mock): mock_entry_point = mock.Mock() - mock_entry_point.load.return_value = TestExtension + mock_entry_point.load.return_value = DummyExtension iter_entry_points_mock.return_value = [mock_entry_point] @@ -94,7 +94,7 @@ class LoadExtensionsTest(object): def test_gets_instance(self, iter_entry_points_mock): mock_entry_point = mock.Mock() - mock_entry_point.load.return_value = TestExtension() + mock_entry_point.load.return_value = DummyExtension() iter_entry_points_mock.return_value = [mock_entry_point] @@ -113,11 +113,11 @@ class LoadExtensionsTest(object): def test_get_config_schema_fails(self, iter_entry_points_mock): mock_entry_point = mock.Mock() - mock_entry_point.load.return_value = TestExtension + mock_entry_point.load.return_value = DummyExtension iter_entry_points_mock.return_value = [mock_entry_point] - with mock.patch.object(TestExtension, 'get_config_schema') as get: + with mock.patch.object(DummyExtension, 'get_config_schema') as get: get.side_effect = Exception assert ext.load_extensions() == [] @@ -125,11 +125,11 @@ class LoadExtensionsTest(object): def test_get_default_config_fails(self, iter_entry_points_mock): mock_entry_point = mock.Mock() - mock_entry_point.load.return_value = TestExtension + mock_entry_point.load.return_value = DummyExtension iter_entry_points_mock.return_value = [mock_entry_point] - with mock.patch.object(TestExtension, 'get_default_config') as get: + with mock.patch.object(DummyExtension, 'get_default_config') as get: get.side_effect = Exception assert ext.load_extensions() == [] @@ -137,22 +137,22 @@ class LoadExtensionsTest(object): def test_get_command_fails(self, iter_entry_points_mock): mock_entry_point = mock.Mock() - mock_entry_point.load.return_value = TestExtension + mock_entry_point.load.return_value = DummyExtension iter_entry_points_mock.return_value = [mock_entry_point] - with mock.patch.object(TestExtension, 'get_command') as get: + with mock.patch.object(DummyExtension, 'get_command') as get: get.side_effect = Exception assert ext.load_extensions() == [] get.assert_called_once_with() -class ValidateExtensionDataTest(object): +class TestValidateExtensionData(object): @pytest.fixture def ext_data(self): - extension = TestExtension() + extension = DummyExtension() entry_point = mock.Mock() entry_point.name = extension.ext_name From b596b85571da38f717cb0c2ea653355077939064 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Sat, 25 Jul 2015 16:24:18 +0200 Subject: [PATCH 19/94] Add "sortname" field in the Artist model (#940) --- docs/changelog.rst | 3 +++ mopidy/audio/utils.py | 23 +++++++++++++++-------- mopidy/models/__init__.py | 3 +++ tests/audio/test_utils.py | 16 +++++++++++++++- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 320c776a..e1174c7d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -47,6 +47,9 @@ Models reuse instances. For the test data set this was developed against, a library of ~14000 tracks, went from needing ~75MB to ~17MB. (Fixes: :issue:`348`) +- Added :attr:`mopidy.models.Artist.sortname` field that is mapped to + ``musicbrainz-sortname`` tag. (Fixes: :issue:`940`) + MPD frontend ------------ diff --git a/mopidy/audio/utils.py b/mopidy/audio/utils.py index 3b9ea30f..a4333b5a 100644 --- a/mopidy/audio/utils.py +++ b/mopidy/audio/utils.py @@ -65,15 +65,21 @@ def supported_uri_schemes(uri_schemes): return supported_schemes -def _artists(tags, artist_name, artist_id=None): +def _artists(tags, artist_name, artist_id=None, artist_sortname=None): # Name missing, don't set artist if not tags.get(artist_name): return None - # One artist name and id, provide artist with id. - if len(tags[artist_name]) == 1 and artist_id in tags: - return [Artist(name=tags[artist_name][0], - musicbrainz_id=tags[artist_id][0])] - # Multiple artist, provide artists without id. + # One artist name and either id or sortname, include all available fields + if len(tags[artist_name]) == 1 and \ + (artist_id in tags or artist_sortname in tags): + attrs = {'name': tags[artist_name][0]} + if artist_id in tags: + attrs['musicbrainz_id'] = tags[artist_id][0] + if artist_sortname in tags: + attrs['sortname'] = tags[artist_sortname][0] + return [Artist(**attrs)] + + # Multiple artist, provide artists with name only to avoid ambiguity. return [Artist(name=name) for name in tags[artist_name]] @@ -91,8 +97,9 @@ def convert_tags_to_track(tags): track_kwargs['composers'] = _artists(tags, gst.TAG_COMPOSER) track_kwargs['performers'] = _artists(tags, gst.TAG_PERFORMER) - track_kwargs['artists'] = _artists( - tags, gst.TAG_ARTIST, 'musicbrainz-artistid') + track_kwargs['artists'] = _artists(tags, gst.TAG_ARTIST, + 'musicbrainz-artistid', + 'musicbrainz-sortname') album_kwargs['artists'] = _artists( tags, gst.TAG_ALBUM_ARTIST, 'musicbrainz-albumartistid') diff --git a/mopidy/models/__init__.py b/mopidy/models/__init__.py index 231a472a..c84254cd 100644 --- a/mopidy/models/__init__.py +++ b/mopidy/models/__init__.py @@ -117,6 +117,9 @@ class Artist(ValidatedImmutableObject): #: The artist name. Read-only. name = fields.String() + #: Artist name for better sorting, e.g. with articles stripped + sortname = fields.String() + #: The MusicBrainz ID of the artist. Read-only. musicbrainz_id = fields.Identifier() diff --git a/tests/audio/test_utils.py b/tests/audio/test_utils.py index 200d7729..0b497dad 100644 --- a/tests/audio/test_utils.py +++ b/tests/audio/test_utils.py @@ -31,11 +31,13 @@ class TagsToTrackTest(unittest.TestCase): 'musicbrainz-trackid': ['trackid'], 'musicbrainz-albumid': ['albumid'], 'musicbrainz-artistid': ['artistid'], + 'musicbrainz-sortname': ['sortname'], 'musicbrainz-albumartistid': ['albumartistid'], 'bitrate': [1000], } - artist = Artist(name='artist', musicbrainz_id='artistid') + artist = Artist(name='artist', musicbrainz_id='artistid', + sortname='sortname') composer = Artist(name='composer') performer = Artist(name='performer') albumartist = Artist(name='albumartist', @@ -245,3 +247,15 @@ class TagsToTrackTest(unittest.TestCase): del self.tags['comment'] self.tags['copyright'] = ['copyright1', 'copyright2'] self.check(self.track.replace(comment='copyright1; copyright2')) + + def test_sortname(self): + self.tags['musicbrainz-sortname'] = ['another_sortname'] + artist = Artist(name='artist', sortname='another_sortname', + musicbrainz_id='artistid') + self.check(self.track.replace(artists=[artist])) + + def test_missing_sortname(self): + del self.tags['musicbrainz-sortname'] + artist = Artist(name='artist', sortname=None, + musicbrainz_id='artistid') + self.check(self.track.replace(artists=[artist])) From 41db4991aa037e515f071a22b8d830bf1bd21f81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dejan=20Proki=C4=87?= Date: Sat, 25 Jul 2015 18:16:35 +0200 Subject: [PATCH 20/94] ext: Add getter methods for cache, config and data directories Methods do get or create directories --- mopidy/ext.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/mopidy/ext.py b/mopidy/ext.py index ab35008a..cdaa5662 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -2,10 +2,12 @@ from __future__ import absolute_import, unicode_literals import collections import logging +import os import pkg_resources from mopidy import config as config_lib, exceptions +from mopidy.internal import path logger = logging.getLogger(__name__) @@ -58,6 +60,42 @@ class Extension(object): schema['enabled'] = config_lib.Boolean() return schema + def get_cache_dir(self, config): + """Get or create cache directory for extension and return path + + :param config: Loaded configuration + :return: string + """ + assert self.ext_name is not None + cache_dir_path = bytes(os.path.join(config['core']['cache_dir'], + self.ext_name)) + path.get_or_create_dir(cache_dir_path) + return cache_dir_path + + def get_config_dir(self, config): + """Get or create configuration directory for extension and return path + + :param config: Loaded configuration + :return: string + """ + assert self.ext_name is not None + config_dir_path = bytes(os.path.join(config['core']['config_dir'], + self.ext_name)) + path.get_or_create_dir(config_dir_path) + return config_dir_path + + def get_data_dir(self, config): + """Get or create data directory for extension and return path + + :param config: Loaded configuration + :returns: string + """ + assert self.ext_name is not None + data_dir_path = bytes(os.path.join(config['core']['data_dir'], + self.ext_name)) + path.get_or_create_dir(data_dir_path) + return data_dir_path + def get_command(self): """Command to expose to command line users running ``mopidy``. From 5f843100cca98167ab350ca49db964c1004ca635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dejan=20Proki=C4=87?= Date: Sat, 25 Jul 2015 18:17:27 +0200 Subject: [PATCH 21/94] tests: Add tests for getter methods for cache, config and data directories --- tests/test_ext.py | 50 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/test_ext.py b/tests/test_ext.py index b73e2bae..1a6bd538 100644 --- a/tests/test_ext.py +++ b/tests/test_ext.py @@ -1,5 +1,7 @@ from __future__ import absolute_import, unicode_literals +import os + import mock import pkg_resources @@ -53,6 +55,21 @@ class TestExtension(object): with pytest.raises(NotImplementedError): extension.setup(None) + def test_get_cache_dir_raises_assertion_error(self, extension): + config = {'core': {'cache_dir': '/tmp'}} + with pytest.raises(AssertionError): # ext_name not set + extension.get_cache_dir(config) + + def test_get_config_dir_raises_assertion_error(self, extension): + config = {'core': {'config_dir': '/tmp'}} + with pytest.raises(AssertionError): # ext_name not set + extension.get_config_dir(config) + + def test_get_data_dir_raises_assertion_error(self, extension): + config = {'core': {'data_dir': '/tmp'}} + with pytest.raises(AssertionError): # ext_name not set + extension.get_data_dir(config) + class TestLoadExtensions(object): @@ -221,3 +238,36 @@ class TestValidateExtensionData(object): def test_no_default_config(self, ext_data): ext_data = ext_data._replace(config_defaults=None) assert not ext.validate_extension_data(ext_data) + + def test_get_cache_dir(self, ext_data): + core_cache_dir = '/tmp' + config = {'core': {'cache_dir': core_cache_dir}} + extension = ext_data.extension + + with mock.patch.object(ext.path, 'get_or_create_dir'): + cache_dir = extension.get_cache_dir(config) + + expected = os.path.join(core_cache_dir, extension.ext_name) + assert cache_dir == expected + + def test_get_config_dir(self, ext_data): + core_config_dir = '/tmp' + config = {'core': {'config_dir': core_config_dir}} + extension = ext_data.extension + + with mock.patch.object(ext.path, 'get_or_create_dir'): + config_dir = extension.get_config_dir(config) + + expected = os.path.join(core_config_dir, extension.ext_name) + assert config_dir == expected + + def test_get_data_dir(self, ext_data): + core_data_dir = '/tmp' + config = {'core': {'data_dir': core_data_dir}} + extension = ext_data.extension + + with mock.patch.object(ext.path, 'get_or_create_dir'): + data_dir = extension.get_data_dir(config) + + expected = os.path.join(core_data_dir, extension.ext_name) + assert data_dir == expected From 2bcbbf03be906395f729c4e06093e327751ab0f4 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 23 Jul 2015 16:36:09 +0200 Subject: [PATCH 22/94] playlists: Simplify detector/parser interface --- mopidy/internal/playlists.py | 26 ++++++++++++-------------- tests/internal/test_playlists.py | 18 ++++-------------- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/mopidy/internal/playlists.py b/mopidy/internal/playlists.py index 34e3ed7a..ace7eaba 100644 --- a/mopidy/internal/playlists.py +++ b/mopidy/internal/playlists.py @@ -14,18 +14,16 @@ except ImportError: import xml.etree.ElementTree as elementtree -# TODO: make detect_FOO_header reusable in general mopidy code. -# i.e. give it just a "peek" like function. -def detect_m3u_header(typefind): - return typefind.peek(0, 7).upper() == b'#EXTM3U' +def detect_m3u_header(data): + return data[0:7].upper() == b'#EXTM3U' -def detect_pls_header(typefind): - return typefind.peek(0, 10).lower() == b'[playlist]' +def detect_pls_header(data): + return data[0:10].lower() == b'[playlist]' -def detect_xspf_header(typefind): - data = typefind.peek(0, 150) +def detect_xspf_header(data): + data = data[0:150] if b'xspf' not in data.lower(): return False @@ -38,8 +36,8 @@ def detect_xspf_header(typefind): return False -def detect_asx_header(typefind): - data = typefind.peek(0, 50) +def detect_asx_header(data): + data = data[0:50] if b'asx' not in data.lower(): return False @@ -55,7 +53,7 @@ def detect_asx_header(typefind): def parse_m3u(data): # TODO: convert non URIs to file URIs. found_header = False - for line in data.readlines(): + for line in data.splitlines(): if found_header or line.startswith(b'#EXTM3U'): found_header = True else: @@ -68,7 +66,7 @@ def parse_pls(data): # TODO: convert non URIs to file URIs. try: cp = configparser.RawConfigParser() - cp.readfp(data) + cp.readfp(io.BytesIO(data)) except configparser.Error: return @@ -82,7 +80,7 @@ def parse_pls(data): def parse_xspf(data): try: # Last element will be root. - for event, element in elementtree.iterparse(data): + for event, element in elementtree.iterparse(io.BytesIO(data)): element.tag = element.tag.lower() # normalize except elementtree.ParseError: return @@ -95,7 +93,7 @@ def parse_xspf(data): def parse_asx(data): try: # Last element will be root. - for event, element in elementtree.iterparse(data): + for event, element in elementtree.iterparse(io.BytesIO(data)): element.tag = element.tag.lower() # normalize except elementtree.ParseError: return diff --git a/tests/internal/test_playlists.py b/tests/internal/test_playlists.py index c5499239..dfe2b71b 100644 --- a/tests/internal/test_playlists.py +++ b/tests/internal/test_playlists.py @@ -2,7 +2,6 @@ from __future__ import absolute_import, unicode_literals -import io import unittest from mopidy.internal import playlists @@ -77,15 +76,6 @@ XSPF = b""" """ -class TypeFind(object): - - def __init__(self, data): - self.data = data - - def peek(self, start, end): - return self.data[start:end] - - class BasePlaylistTest(object): valid = None invalid = None @@ -93,18 +83,18 @@ class BasePlaylistTest(object): parse = None def test_detect_valid_header(self): - self.assertTrue(self.detect(TypeFind(self.valid))) + self.assertTrue(self.detect(self.valid)) def test_detect_invalid_header(self): - self.assertFalse(self.detect(TypeFind(self.invalid))) + self.assertFalse(self.detect(self.invalid)) def test_parse_valid_playlist(self): - uris = list(self.parse(io.BytesIO(self.valid))) + uris = list(self.parse(self.valid)) expected = [b'file:///tmp/foo', b'file:///tmp/bar', b'file:///tmp/baz'] self.assertEqual(uris, expected) def test_parse_invalid_playlist(self): - uris = list(self.parse(io.BytesIO(self.invalid))) + uris = list(self.parse(self.invalid)) self.assertEqual(uris, []) From 6af11b563f17ea50091119649388adf43b52073d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 23 Jul 2015 16:36:35 +0200 Subject: [PATCH 23/94] playlists: Add parse() that detects format and parses --- mopidy/internal/playlists.py | 13 +++++++++++++ tests/internal/test_playlists.py | 19 +++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/mopidy/internal/playlists.py b/mopidy/internal/playlists.py index ace7eaba..f339b357 100644 --- a/mopidy/internal/playlists.py +++ b/mopidy/internal/playlists.py @@ -14,6 +14,19 @@ except ImportError: import xml.etree.ElementTree as elementtree +def parse(data): + handlers = { + detect_m3u_header: parse_m3u, + detect_pls_header: parse_pls, + detect_asx_header: parse_asx, + detect_xspf_header: parse_xspf, + } + for detector, parser in handlers.items(): + if detector(data): + return list(parser(data)) + return [] + + def detect_m3u_header(data): return data[0:7].upper() == b'#EXTM3U' diff --git a/tests/internal/test_playlists.py b/tests/internal/test_playlists.py index dfe2b71b..c53a33af 100644 --- a/tests/internal/test_playlists.py +++ b/tests/internal/test_playlists.py @@ -4,6 +4,8 @@ from __future__ import absolute_import, unicode_literals import unittest +import pytest + from mopidy.internal import playlists @@ -75,6 +77,20 @@ XSPF = b""" """ +EXPECTED = [b'file:///tmp/foo', b'file:///tmp/bar', b'file:///tmp/baz'] + + +@pytest.mark.parametrize('data,result', [ + (BAD, []), + (M3U, EXPECTED), + (PLS, EXPECTED), + (ASX, EXPECTED), + (SIMPLE_ASX, EXPECTED), + (XSPF, EXPECTED), +]) +def test_parse(data, result): + assert playlists.parse(data) == result + class BasePlaylistTest(object): valid = None @@ -90,8 +106,7 @@ class BasePlaylistTest(object): def test_parse_valid_playlist(self): uris = list(self.parse(self.valid)) - expected = [b'file:///tmp/foo', b'file:///tmp/bar', b'file:///tmp/baz'] - self.assertEqual(uris, expected) + self.assertEqual(uris, EXPECTED) def test_parse_invalid_playlist(self): uris = list(self.parse(self.invalid)) From 12c27142a37b96639cd2bf1a8f593c892134f798 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 25 Jul 2015 19:11:29 +0200 Subject: [PATCH 24/94] playlists: Rename M3U to EXTM3U --- mopidy/internal/playlists.py | 6 +++--- tests/internal/test_playlists.py | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/mopidy/internal/playlists.py b/mopidy/internal/playlists.py index f339b357..4d01556e 100644 --- a/mopidy/internal/playlists.py +++ b/mopidy/internal/playlists.py @@ -16,7 +16,7 @@ except ImportError: def parse(data): handlers = { - detect_m3u_header: parse_m3u, + detect_extm3u_header: parse_extm3u, detect_pls_header: parse_pls, detect_asx_header: parse_asx, detect_xspf_header: parse_xspf, @@ -27,7 +27,7 @@ def parse(data): return [] -def detect_m3u_header(data): +def detect_extm3u_header(data): return data[0:7].upper() == b'#EXTM3U' @@ -63,7 +63,7 @@ def detect_asx_header(data): return False -def parse_m3u(data): +def parse_extm3u(data): # TODO: convert non URIs to file URIs. found_header = False for line in data.splitlines(): diff --git a/tests/internal/test_playlists.py b/tests/internal/test_playlists.py index c53a33af..d4102027 100644 --- a/tests/internal/test_playlists.py +++ b/tests/internal/test_playlists.py @@ -11,7 +11,7 @@ from mopidy.internal import playlists BAD = b'foobarbaz' -M3U = b"""#EXTM3U +EXTM3U = b"""#EXTM3U #EXTINF:123, Sample artist - Sample title file:///tmp/foo #EXTINF:321,Example Artist - Example \xc5\xa7\xc5\x95 @@ -82,7 +82,7 @@ EXPECTED = [b'file:///tmp/foo', b'file:///tmp/bar', b'file:///tmp/baz'] @pytest.mark.parametrize('data,result', [ (BAD, []), - (M3U, EXPECTED), + (EXTM3U, EXPECTED), (PLS, EXPECTED), (ASX, EXPECTED), (SIMPLE_ASX, EXPECTED), @@ -113,11 +113,11 @@ class BasePlaylistTest(object): self.assertEqual(uris, []) -class M3uPlaylistTest(BasePlaylistTest, unittest.TestCase): - valid = M3U +class ExtM3uPlaylistTest(BasePlaylistTest, unittest.TestCase): + valid = EXTM3U invalid = BAD - detect = staticmethod(playlists.detect_m3u_header) - parse = staticmethod(playlists.parse_m3u) + detect = staticmethod(playlists.detect_extm3u_header) + parse = staticmethod(playlists.parse_extm3u) class PlsPlaylistTest(BasePlaylistTest, unittest.TestCase): From 80adc100ae565c386db53d65079f9e35edc7d83b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 25 Jul 2015 19:13:14 +0200 Subject: [PATCH 25/94] playlists: Add blank lines to valid playlist data --- tests/internal/test_playlists.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/internal/test_playlists.py b/tests/internal/test_playlists.py index d4102027..4550ea3d 100644 --- a/tests/internal/test_playlists.py +++ b/tests/internal/test_playlists.py @@ -16,6 +16,7 @@ EXTM3U = b"""#EXTM3U file:///tmp/foo #EXTINF:321,Example Artist - Example \xc5\xa7\xc5\x95 file:///tmp/bar + #EXTINF:213,Some Artist - Other title file:///tmp/baz """ @@ -25,6 +26,7 @@ NumberOfEntries=3 File1=file:///tmp/foo Title1=Sample Title Length1=123 + File2=file:///tmp/bar Title2=Example \xc5\xa7\xc5\x95 Length2=321 From ea2017c9685fd5359428ce7e27d2070f3adfb36e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 25 Jul 2015 19:13:37 +0200 Subject: [PATCH 26/94] playlists: Add urilist fallback parser --- mopidy/internal/playlists.py | 16 +++++++++++++++- tests/internal/test_playlists.py | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/mopidy/internal/playlists.py b/mopidy/internal/playlists.py index 4d01556e..219d3ec6 100644 --- a/mopidy/internal/playlists.py +++ b/mopidy/internal/playlists.py @@ -7,6 +7,7 @@ pygst.require('0.10') import gst # noqa from mopidy.compat import configparser +from mopidy.internal import validation try: import xml.etree.cElementTree as elementtree @@ -24,7 +25,7 @@ def parse(data): for detector, parser in handlers.items(): if detector(data): return list(parser(data)) - return [] + return parse_urilist(data) # Fallback def detect_extm3u_header(data): @@ -116,3 +117,16 @@ def parse_asx(data): for entry in element.findall('entry[@href]'): yield entry.get('href', '').strip() + + +def parse_urilist(data): + result = [] + for line in data.splitlines(): + if not line.strip() or line.startswith('#'): + continue + try: + validation.check_uri(line) + except ValueError: + return [] + result.append(line) + return result diff --git a/tests/internal/test_playlists.py b/tests/internal/test_playlists.py index 4550ea3d..21537813 100644 --- a/tests/internal/test_playlists.py +++ b/tests/internal/test_playlists.py @@ -21,6 +21,14 @@ file:///tmp/bar file:///tmp/baz """ +URILIST = b""" +file:///tmp/foo +# a comment +file:///tmp/bar + +file:///tmp/baz +""" + PLS = b"""[Playlist] NumberOfEntries=3 File1=file:///tmp/foo @@ -84,6 +92,7 @@ EXPECTED = [b'file:///tmp/foo', b'file:///tmp/bar', b'file:///tmp/baz'] @pytest.mark.parametrize('data,result', [ (BAD, []), + (URILIST, EXPECTED), (EXTM3U, EXPECTED), (PLS, EXPECTED), (ASX, EXPECTED), @@ -148,3 +157,17 @@ class XspfPlaylistTest(BasePlaylistTest, unittest.TestCase): invalid = BAD detect = staticmethod(playlists.detect_xspf_header) parse = staticmethod(playlists.parse_xspf) + + +class UriListPlaylistTest(unittest.TestCase): + valid = URILIST + invalid = BAD + parse = staticmethod(playlists.parse_urilist) + + def test_parse_valid_playlist(self): + uris = list(self.parse(self.valid)) + self.assertEqual(uris, EXPECTED) + + def test_parse_invalid_playlist(self): + uris = list(self.parse(self.invalid)) + self.assertEqual(uris, []) From fcb61e3551f6fb1e3f3b2d9ac516e6ec919f8216 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 26 Jul 2015 00:09:33 +0200 Subject: [PATCH 27/94] backend: Log translated URIs --- mopidy/backend.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/mopidy/backend.py b/mopidy/backend.py index fd91044f..8d7a831e 100644 --- a/mopidy/backend.py +++ b/mopidy/backend.py @@ -1,8 +1,13 @@ from __future__ import absolute_import, unicode_literals +import logging + from mopidy import listener, models +logger = logging.getLogger(__name__) + + class Backend(object): """Backend API @@ -238,6 +243,9 @@ class PlaybackProvider(object): :rtype: :class:`True` if successful, else :class:`False` """ uri = self.translate_uri(track.uri) + if uri != track.uri: + logger.debug( + 'Backend translated URI from %s to %s', track.uri, uri) if not uri: return False self.audio.set_uri(uri).get() From f373d071ea2b2786615b9f1dd7ed0c1aabd90719 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 23 Jul 2015 14:45:23 +0200 Subject: [PATCH 28/94] stream: Move scanner to backend, port tests to pytest --- mopidy/stream/actor.py | 12 ++++--- tests/stream/test_library.py | 65 +++++++++++++++++++++--------------- 2 files changed, 45 insertions(+), 32 deletions(-) diff --git a/mopidy/stream/actor.py b/mopidy/stream/actor.py index 4b81f60e..b70ce360 100644 --- a/mopidy/stream/actor.py +++ b/mopidy/stream/actor.py @@ -20,21 +20,23 @@ class StreamBackend(pykka.ThreadingActor, backend.Backend): super(StreamBackend, self).__init__() self.library = StreamLibraryProvider( - backend=self, timeout=config['stream']['timeout'], - blacklist=config['stream']['metadata_blacklist'], - proxy=config['proxy']) + backend=self, blacklist=config['stream']['metadata_blacklist']) self.playback = backend.PlaybackProvider(audio=audio, backend=self) self.playlists = None self.uri_schemes = audio_lib.supported_uri_schemes( config['stream']['protocols']) + self._scanner = scan.Scanner( + timeout=config['stream']['timeout'], + proxy_config=config['proxy']) + class StreamLibraryProvider(backend.LibraryProvider): - def __init__(self, backend, timeout, blacklist, proxy): + def __init__(self, backend, blacklist): super(StreamLibraryProvider, self).__init__(backend) - self._scanner = scan.Scanner(timeout=timeout, proxy_config=proxy) + self._scanner = backend._scanner self._blacklist_re = re.compile( r'^(%s)$' % '|'.join(fnmatch.translate(u) for u in blacklist)) diff --git a/tests/stream/test_library.py b/tests/stream/test_library.py index 3962159c..67053924 100644 --- a/tests/stream/test_library.py +++ b/tests/stream/test_library.py @@ -1,16 +1,10 @@ from __future__ import absolute_import, unicode_literals -import unittest - -import gobject -gobject.threads_init() - import mock -import pygst -pygst.require('0.10') -import gst # noqa: pygst magic is needed to import correct gst +import pytest +from mopidy.audio import scan from mopidy.internal import path from mopidy.models import Track from mopidy.stream import actor @@ -18,27 +12,44 @@ from mopidy.stream import actor from tests import path_to_data_dir -class LibraryProviderTest(unittest.TestCase): +@pytest.fixture +def scanner(): + return scan.Scanner(timeout=100, proxy_config={}) - def setUp(self): # noqa: N802 - self.backend = mock.Mock() - self.backend.uri_schemes = ['file'] - self.uri = path.path_to_uri(path_to_data_dir('song1.wav')) - def test_lookup_ignores_unknown_scheme(self): - library = actor.StreamLibraryProvider(self.backend, 1000, [], {}) - self.assertFalse(library.lookup('http://example.com')) +@pytest.fixture +def backend(scanner): + backend = mock.Mock() + backend.uri_schemes = ['file'] + backend._scanner = scanner + return backend - def test_lookup_respects_blacklist(self): - library = actor.StreamLibraryProvider(self.backend, 10, [self.uri], {}) - self.assertEqual([Track(uri=self.uri)], library.lookup(self.uri)) - def test_lookup_respects_blacklist_globbing(self): - blacklist = [path.path_to_uri(path_to_data_dir('')) + '*'] - library = actor.StreamLibraryProvider(self.backend, 100, blacklist, {}) - self.assertEqual([Track(uri=self.uri)], library.lookup(self.uri)) +@pytest.fixture +def track_uri(): + return path.path_to_uri(path_to_data_dir('song1.wav')) - def test_lookup_converts_uri_metadata_to_track(self): - library = actor.StreamLibraryProvider(self.backend, 100, [], {}) - self.assertEqual([Track(length=4406, uri=self.uri)], - library.lookup(self.uri)) + +def test_lookup_ignores_unknown_scheme(backend): + library = actor.StreamLibraryProvider(backend, []) + + assert library.lookup('http://example.com') == [] + + +def test_lookup_respects_blacklist(backend, track_uri): + library = actor.StreamLibraryProvider(backend, [track_uri]) + + assert library.lookup(track_uri) == [Track(uri=track_uri)] + + +def test_lookup_respects_blacklist_globbing(backend, track_uri): + blacklist = [path.path_to_uri(path_to_data_dir('')) + '*'] + library = actor.StreamLibraryProvider(backend, blacklist) + + assert library.lookup(track_uri) == [Track(uri=track_uri)] + + +def test_lookup_converts_uri_metadata_to_track(backend, track_uri): + library = actor.StreamLibraryProvider(backend, []) + + assert library.lookup(track_uri) == [Track(length=4406, uri=track_uri)] From 16f80ccb8dc2eb4c825aa4e9a0664889d31c5183 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 23 Jul 2015 15:24:28 +0200 Subject: [PATCH 29/94] Add a dependency on Requests And on Responses for mocking Requests calls in tests. --- dev-requirements.txt | 1 + docs/changelog.rst | 5 +++++ setup.py | 3 ++- tox.ini | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index eba66348..3b96a76a 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -10,6 +10,7 @@ flake8-import-order # Mock dependencies in tests mock +responses # Test runners pytest diff --git a/docs/changelog.rst b/docs/changelog.rst index 320c776a..914d7443 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,6 +7,11 @@ This changelog is used to track all major changes to Mopidy. v1.1.0 (UNRELEASED) =================== +Dependencies +------------ + +- Mopidy now requires Requests. + Core API -------- diff --git a/setup.py b/setup.py index ca121f74..ba74179c 100644 --- a/setup.py +++ b/setup.py @@ -24,8 +24,9 @@ setup( zip_safe=False, include_package_data=True, install_requires=[ - 'setuptools', 'Pykka >= 1.1', + 'requests', + 'setuptools', 'tornado >= 2.3', ], extras_require={'http': []}, diff --git a/tox.ini b/tox.ini index 6dfab5ae..c4afdacd 100644 --- a/tox.ini +++ b/tox.ini @@ -15,6 +15,7 @@ deps = pytest pytest-cov pytest-xdist + responses [testenv:py27-tornado23] commands = py.test tests/http From c4faf37bf4f6ad47d296e6f8c1d46593bfcb0980 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 26 Jul 2015 11:24:31 +0200 Subject: [PATCH 30/94] Add a dev dependency on pytest-capturelog --- dev-requirements.txt | 1 + tox.ini | 1 + 2 files changed, 2 insertions(+) diff --git a/dev-requirements.txt b/dev-requirements.txt index 3b96a76a..809a0038 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -14,6 +14,7 @@ responses # Test runners pytest +pytest-capturelog pytest-cov pytest-xdist tox diff --git a/tox.ini b/tox.ini index c4afdacd..e29a40f2 100644 --- a/tox.ini +++ b/tox.ini @@ -13,6 +13,7 @@ commands = deps = mock pytest + pytest-capturelog pytest-cov pytest-xdist responses From d991e51d40c981de6178a35b4646eee6af42f6c1 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 23 Jul 2015 15:50:27 +0200 Subject: [PATCH 31/94] stream: Extract first track from playlists --- docs/changelog.rst | 6 ++ mopidy/stream/actor.py | 92 +++++++++++++++++++-- tests/stream/test_playback.py | 145 ++++++++++++++++++++++++++++++++++ 3 files changed, 237 insertions(+), 6 deletions(-) create mode 100644 tests/stream/test_playback.py diff --git a/docs/changelog.rst b/docs/changelog.rst index 914d7443..a5066270 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -85,6 +85,12 @@ MPD frontend - Track data now include the ``Last-Modified`` field if set on the track model. (Fixes: :issue:`1218`, PR: :issue:`1219`) +Stream backend +-------------- + +- Move stream playlist parsing from GStreamer to the stream backend. (Fixes: + :issue:`671`) + Local backend ------------- diff --git a/mopidy/stream/actor.py b/mopidy/stream/actor.py index b70ce360..a6040f65 100644 --- a/mopidy/stream/actor.py +++ b/mopidy/stream/actor.py @@ -3,12 +3,16 @@ from __future__ import absolute_import, unicode_literals import fnmatch import logging import re +import time import urlparse import pykka -from mopidy import audio as audio_lib, backend, exceptions +import requests + +from mopidy import audio as audio_lib, backend, exceptions, httpclient, stream from mopidy.audio import scan, utils +from mopidy.internal import playlists from mopidy.models import Track logger = logging.getLogger(__name__) @@ -19,18 +23,19 @@ class StreamBackend(pykka.ThreadingActor, backend.Backend): def __init__(self, config, audio): super(StreamBackend, self).__init__() + self._scanner = scan.Scanner( + timeout=config['stream']['timeout'], + proxy_config=config['proxy']) + self.library = StreamLibraryProvider( backend=self, blacklist=config['stream']['metadata_blacklist']) - self.playback = backend.PlaybackProvider(audio=audio, backend=self) + self.playback = StreamPlaybackProvider( + audio=audio, backend=self, config=config) self.playlists = None self.uri_schemes = audio_lib.supported_uri_schemes( config['stream']['protocols']) - self._scanner = scan.Scanner( - timeout=config['stream']['timeout'], - proxy_config=config['proxy']) - class StreamLibraryProvider(backend.LibraryProvider): @@ -57,3 +62,78 @@ class StreamLibraryProvider(backend.LibraryProvider): track = Track(uri=uri) return [track] + + +class StreamPlaybackProvider(backend.PlaybackProvider): + + def __init__(self, audio, backend, config): + super(StreamPlaybackProvider, self).__init__(audio, backend) + self._config = config + self._scanner = backend._scanner + + def translate_uri(self, uri): + try: + scan_result = self._scanner.scan(uri) + except exceptions.ScannerError as e: + logger.warning( + 'Problem scanning URI %s: %s', uri, e) + return None + + if not (scan_result.mime.startswith('text/') or + scan_result.mime.startswith('application/')): + return uri + + content = self._download(uri) + if content is None: + return None + + tracks = list(playlists.parse(content)) + if tracks: + # TODO Test streams and return first that seems to be playable + return tracks[0] + + def _download(self, uri): + timeout = self._config['stream']['timeout'] / 1000.0 + + session = get_requests_session( + proxy_config=self._config['proxy'], + user_agent='%s/%s' % ( + stream.Extension.dist_name, stream.Extension.version)) + + try: + response = session.get( + uri, stream=True, timeout=timeout) + except requests.exceptions.Timeout: + logger.warning( + 'Download of stream playlist (%s) failed due to connection ' + 'timeout after %.3fs', uri, timeout) + return None + + deadline = time.time() + timeout + content = b'' + for chunk in response.iter_content(4096): + content += chunk + if time.time() > deadline: + logger.warning( + 'Download of stream playlist (%s) failed due to download ' + 'taking more than %.3fs', uri, timeout) + return None + + if not response.ok: + logger.warning( + 'Problem downloading stream playlist %s: %s', + uri, response.reason) + return None + + return content + + +def get_requests_session(proxy_config, user_agent): + proxy = httpclient.format_proxy(proxy_config) + full_user_agent = httpclient.format_user_agent(user_agent) + + session = requests.Session() + session.proxies.update({'http': proxy, 'https': proxy}) + session.headers.update({'user-agent': full_user_agent}) + + return session diff --git a/tests/stream/test_playback.py b/tests/stream/test_playback.py new file mode 100644 index 00000000..4da87ae0 --- /dev/null +++ b/tests/stream/test_playback.py @@ -0,0 +1,145 @@ +from __future__ import absolute_import, unicode_literals + +import mock + +import pytest + +import requests + +import responses + +from mopidy import exceptions +from mopidy.audio import scan +from mopidy.stream import actor + + +TIMEOUT = 1000 +URI = 'http://example.com/listen.m3u' +BODY = """ +#EXTM3U +http://example.com/stream.mp3 +http://foo.bar/baz +""".strip() + + +@pytest.fixture +def config(): + return { + 'proxy': {}, + 'stream': { + 'timeout': TIMEOUT, + }, + } + + +@pytest.fixture +def audio(): + return mock.Mock() + + +@pytest.fixture +def scanner(): + scanner = mock.Mock(spec=scan.Scanner) + scanner.scan.return_value.mime = 'text/foo' + return scanner + + +@pytest.fixture +def backend(scanner): + backend = mock.Mock() + backend.uri_schemes = ['file'] + backend._scanner = scanner + return backend + + +@pytest.fixture +def provider(audio, backend, config): + return actor.StreamPlaybackProvider(audio, backend, config) + + +@responses.activate +def test_translate_uri_of_audio_stream_returns_same_uri( + scanner, provider): + + scanner.scan.return_value.mime = 'audio/ogg' + + result = provider.translate_uri(URI) + + scanner.scan.assert_called_once_with(URI) + assert result == URI + + +@responses.activate +def test_translate_uri_of_playlist_returns_first_uri_in_list( + scanner, provider): + + responses.add( + responses.GET, URI, body=BODY, content_type='audio/x-mpegurl') + + result = provider.translate_uri(URI) + + scanner.scan.assert_called_once_with(URI) + assert result == 'http://example.com/stream.mp3' + assert responses.calls[0].request.headers['User-Agent'].startswith( + 'Mopidy-Stream/') + + +@responses.activate +def test_translate_uri_of_playlist_with_xml_mimetype(scanner, provider): + scanner.scan.return_value.mime = 'application/xspf+xml' + responses.add( + responses.GET, URI, body=BODY, content_type='application/xspf+xml') + + result = provider.translate_uri(URI) + + scanner.scan.assert_called_once_with(URI) + assert result == 'http://example.com/stream.mp3' + + +def test_translate_uri_when_scanner_fails(scanner, provider, caplog): + scanner.scan.side_effect = exceptions.ScannerError('foo failed') + + result = provider.translate_uri('bar') + + assert result is None + assert 'Problem scanning URI bar: foo failed' in caplog.text() + + +@responses.activate +def test_translate_uri_when_playlist_download_fails(provider, caplog): + responses.add(responses.GET, URI, body=BODY, status=500) + + result = provider.translate_uri(URI) + + assert result is None + assert 'Problem downloading stream playlist' in caplog.text() + + +def test_translate_uri_times_out_if_connection_times_out(provider, caplog): + with mock.patch.object(actor.requests, 'Session') as session_mock: + get_mock = session_mock.return_value.get + get_mock.side_effect = requests.exceptions.Timeout + + result = provider.translate_uri(URI) + + get_mock.assert_called_once_with(URI, timeout=1.0, stream=True) + assert result is None + assert ( + 'Download of stream playlist (%s) failed due to connection ' + 'timeout after 1.000s' % URI in caplog.text()) + + +@responses.activate +def test_translate_uri_times_out_if_download_is_slow(provider, caplog): + responses.add( + responses.GET, URI, body=BODY, content_type='audio/x-mpegurl') + + with mock.patch.object(actor, 'time') as time_mock: + time_mock.time.side_effect = [0, TIMEOUT + 1] + + result = provider.translate_uri(URI) + + assert result is None + assert ( + 'Download of stream playlist (%s) failed due to download taking ' + 'more than 1.000s' % URI in caplog.text()) From b5654f478390f401c1375eb234a574a1d269a341 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 26 Jul 2015 10:39:39 +0200 Subject: [PATCH 32/94] stream: Move requests session helper to internal --- mopidy/internal/http.py | 16 ++++++++++++++++ mopidy/stream/actor.py | 17 +++-------------- 2 files changed, 19 insertions(+), 14 deletions(-) create mode 100644 mopidy/internal/http.py diff --git a/mopidy/internal/http.py b/mopidy/internal/http.py new file mode 100644 index 00000000..6ff59590 --- /dev/null +++ b/mopidy/internal/http.py @@ -0,0 +1,16 @@ +from __future__ import absolute_import, unicode_literals + +import requests + +from mopidy import httpclient + + +def get_requests_session(proxy_config, user_agent): + proxy = httpclient.format_proxy(proxy_config) + full_user_agent = httpclient.format_user_agent(user_agent) + + session = requests.Session() + session.proxies.update({'http': proxy, 'https': proxy}) + session.headers.update({'user-agent': full_user_agent}) + + return session diff --git a/mopidy/stream/actor.py b/mopidy/stream/actor.py index a6040f65..069965fb 100644 --- a/mopidy/stream/actor.py +++ b/mopidy/stream/actor.py @@ -10,9 +10,9 @@ import pykka import requests -from mopidy import audio as audio_lib, backend, exceptions, httpclient, stream +from mopidy import audio as audio_lib, backend, exceptions, stream from mopidy.audio import scan, utils -from mopidy.internal import playlists +from mopidy.internal import http, playlists from mopidy.models import Track logger = logging.getLogger(__name__) @@ -95,7 +95,7 @@ class StreamPlaybackProvider(backend.PlaybackProvider): def _download(self, uri): timeout = self._config['stream']['timeout'] / 1000.0 - session = get_requests_session( + session = http.get_requests_session( proxy_config=self._config['proxy'], user_agent='%s/%s' % ( stream.Extension.dist_name, stream.Extension.version)) @@ -126,14 +126,3 @@ class StreamPlaybackProvider(backend.PlaybackProvider): return None return content - - -def get_requests_session(proxy_config, user_agent): - proxy = httpclient.format_proxy(proxy_config) - full_user_agent = httpclient.format_user_agent(user_agent) - - session = requests.Session() - session.proxies.update({'http': proxy, 'https': proxy}) - session.headers.update({'user-agent': full_user_agent}) - - return session From 5f7dded4a3a16efbfbe60bdaa7edfd6d19fe2967 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 26 Jul 2015 11:11:02 +0200 Subject: [PATCH 33/94] stream: Faster content buffer building --- mopidy/stream/actor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/stream/actor.py b/mopidy/stream/actor.py index 069965fb..ae5be8e0 100644 --- a/mopidy/stream/actor.py +++ b/mopidy/stream/actor.py @@ -110,9 +110,9 @@ class StreamPlaybackProvider(backend.PlaybackProvider): return None deadline = time.time() + timeout - content = b'' + content = [] for chunk in response.iter_content(4096): - content += chunk + content.append(chunk) if time.time() > deadline: logger.warning( 'Download of stream playlist (%s) failed due to download ' @@ -125,4 +125,4 @@ class StreamPlaybackProvider(backend.PlaybackProvider): uri, response.reason) return None - return content + return b''.join(content) From 8975e72b349e5d87e5ddcd8b4a99659562033a97 Mon Sep 17 00:00:00 2001 From: Danilo Bargen Date: Sat, 25 Jul 2015 18:01:56 +0200 Subject: [PATCH 34/94] Implemented playlist_deleted event --- docs/changelog.rst | 2 ++ mopidy/core/listener.py | 11 +++++++++++ mopidy/core/playlists.py | 5 +++-- mopidy/m3u/playlists.py | 1 + tests/core/test_events.py | 7 ++++--- tests/core/test_listener.py | 3 +++ 6 files changed, 24 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 320c776a..e5b478d2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -36,6 +36,8 @@ Core API - Add `max_tracklist_length` config and limitation. (Fixes: :issue:`997` PR: :issue:`1225`) +- Added ``playlist_deleted`` event (Fixes: :issue:`996`) + Models ------ diff --git a/mopidy/core/listener.py b/mopidy/core/listener.py index 45109bba..79040052 100644 --- a/mopidy/core/listener.py +++ b/mopidy/core/listener.py @@ -123,6 +123,17 @@ class CoreListener(listener.Listener): """ pass + def playlist_deleted(self, uri): + """ + Called whenever a playlist is deleted. + + *MAY* be implemented by actor. + + :param uri: the uri of the deleted playlist + :type uri: string + """ + pass + def options_changed(self): """ Called whenever an option is changed. diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 086806cc..0ea78f26 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -178,11 +178,12 @@ class PlaylistsController(object): uri_scheme = urlparse.urlparse(uri).scheme backend = self.backends.with_playlists.get(uri_scheme, None) if not backend: - return + return None # TODO: error reporting to user with _backend_error_handling(backend): backend.playlists.delete(uri).get() - # TODO: emit playlist changed? + # TODO: error detection and reporting to user + listener.CoreListener.send('playlist_deleted', uri=uri) # TODO: return value? diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index bd8b8bfd..b1e1bfe8 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -54,6 +54,7 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): logger.warning( 'Trying to delete missing playlist file %s', path) del self._playlists[uri] + logger.info('Deleted playlist %s', uri) else: logger.warning('Trying to delete unknown playlist %s', uri) diff --git a/tests/core/test_events.py b/tests/core/test_events.py index 9a439084..be47d506 100644 --- a/tests/core/test_events.py +++ b/tests/core/test_events.py @@ -99,10 +99,11 @@ class BackendEventsTest(unittest.TestCase): self.assertEqual(send.call_args[0][0], 'playlist_changed') - @unittest.SkipTest def test_playlists_delete_sends_playlist_deleted_event(self, send): - # TODO We should probably add a playlist_deleted event - pass + playlist = self.core.playlists.create('foo').get() + self.core.playlists.delete(playlist.uri).get() + + self.assertEqual(send.call_args[0][0], 'playlist_deleted') def test_playlists_save_sends_playlist_changed_event(self, send): playlist = self.core.playlists.create('foo').get() diff --git a/tests/core/test_listener.py b/tests/core/test_listener.py index 95c4da51..f78b061b 100644 --- a/tests/core/test_listener.py +++ b/tests/core/test_listener.py @@ -47,6 +47,9 @@ class CoreListenerTest(unittest.TestCase): def test_listener_has_default_impl_for_playlist_changed(self): self.listener.playlist_changed(Playlist()) + def test_listener_has_default_impl_for_playlist_deleted(self): + self.listener.playlist_deleted(Playlist()) + def test_listener_has_default_impl_for_options_changed(self): self.listener.options_changed() From 70cfc0b33a2585208648e518c2debf3ad58ee216 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 26 Jul 2015 12:12:15 +0200 Subject: [PATCH 35/94] mpd: PR #1235 tweaks, add changelog --- docs/changelog.rst | 5 +++++ mopidy/mpd/translator.py | 13 ++++--------- tests/mpd/protocol/test_reflection.py | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 320c776a..8637ac1f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -80,6 +80,11 @@ MPD frontend - Track data now include the ``Last-Modified`` field if set on the track model. (Fixes: :issue:`1218`, PR: :issue:`1219`) +- Implement ``tagtypes`` MPD command. (PR: :issue:`1235`) + +- Exclude empty tags fields from metadata output. (Fixes: :issue:`1045`, PR: + :issue:`1235`) + Local backend ------------- diff --git a/mopidy/mpd/translator.py b/mopidy/mpd/translator.py index 2f3efd1d..ef16ed28 100644 --- a/mopidy/mpd/translator.py +++ b/mopidy/mpd/translator.py @@ -97,7 +97,6 @@ def track_to_mpd_format(track, position=None, stream_title=None): if track.musicbrainz_id is not None: result.append(('MUSICBRAINZ_TRACKID', track.musicbrainz_id)) - # clean up result result = [element for element in result if _has_value(*element)] return result @@ -105,19 +104,15 @@ def track_to_mpd_format(track, position=None, stream_title=None): def _has_value(tagtype, value): """ - Determine whether to add the tagtype - to the output or not (if they have a default value). + Determine whether to add the tagtype to the output or not. - :param tagtype: the mpd tagtype + :param tagtype: the MPD tagtype :type tagtype: string - :param value: the associated value - :type value: multiple + :param value: the tag value :rtype: bool """ if tagtype in tagtype_list.TAGTYPE_LIST: - if not value: - return False - + return bool(value) return True diff --git a/tests/mpd/protocol/test_reflection.py b/tests/mpd/protocol/test_reflection.py index 0745479d..097c2e2a 100644 --- a/tests/mpd/protocol/test_reflection.py +++ b/tests/mpd/protocol/test_reflection.py @@ -41,7 +41,6 @@ class ReflectionHandlerTest(protocol.BaseTestCase): def test_tagtypes(self): self.send_request('tagtypes') - self.assertInResponse('OK') self.assertInResponse('tagtype: Artist') self.assertInResponse('tagtype: ArtistSort') self.assertInResponse('tagtype: Album') @@ -59,6 +58,7 @@ class ReflectionHandlerTest(protocol.BaseTestCase): self.assertInResponse('tagtype: MUSICBRAINZ_ALBUMID') self.assertInResponse('tagtype: MUSICBRAINZ_ALBUMARTISTID') self.assertInResponse('tagtype: MUSICBRAINZ_TRACKID') + self.assertInResponse('OK') def test_urlhandlers(self): self.send_request('urlhandlers') From e930e2becdacdc4739a05948c19ad474ce63ccb6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 26 Jul 2015 12:18:49 +0200 Subject: [PATCH 36/94] core: Tweak PR #1238 --- docs/changelog.rst | 2 +- mopidy/core/listener.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index f8a067bf..8648dd8c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -41,7 +41,7 @@ Core API - Add `max_tracklist_length` config and limitation. (Fixes: :issue:`997` PR: :issue:`1225`) -- Added ``playlist_deleted`` event (Fixes: :issue:`996`) +- Added ``playlist_deleted`` event. (Fixes: :issue:`996`) Models ------ diff --git a/mopidy/core/listener.py b/mopidy/core/listener.py index 79040052..d95bd491 100644 --- a/mopidy/core/listener.py +++ b/mopidy/core/listener.py @@ -129,7 +129,7 @@ class CoreListener(listener.Listener): *MAY* be implemented by actor. - :param uri: the uri of the deleted playlist + :param uri: the URI of the deleted playlist :type uri: string """ pass From 0fcf492327020fd82f2258fa90e4318a413751df Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 26 Jul 2015 12:55:16 +0200 Subject: [PATCH 37/94] docs: Tweak #1232 docs, update changelog --- docs/changelog.rst | 21 +++++++++++++++++++++ docs/config.rst | 24 +++++++++++++++++++++--- mopidy/ext.py | 12 ++++++------ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8648dd8c..fb587c6c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -57,6 +57,27 @@ Models - Added :attr:`mopidy.models.Artist.sortname` field that is mapped to ``musicbrainz-sortname`` tag. (Fixes: :issue:`940`) +Configuration +------------- + +- Add new configurations to set base directories to be used by Mopidy and + Mopidy extensions: :confval:`core/cache_dir`, :confval:`core/config_dir`, and + :confval:`core/data_dir`. (Fixes: :issue:`843`, PR: :issue:`1232`) + +Extension support +----------------- + +- Add new methods to :class:`~mopidy.ext.Extension` class for getting cache, + config and data dirs specific to your extension: + + - :meth:`mopidy.ext.Extension.get_cache_dir` + - :meth:`mopidy.ext.Extension.get_config_dir` + - :meth:`mopidy.ext.Extension.get_data_dir` + + Extensions should use these methods so that the correct directories are used + both when Mopidy is run by a regular user and when run as a system service. + (Fixes: :issue:`843`, PR: :issue:`1232`) + MPD frontend ------------ diff --git a/docs/config.rst b/docs/config.rst index b6be6cfa..26304176 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -63,15 +63,33 @@ Core configuration .. confval:: core/cache_dir - Path to directory for storing cached information. + Path to base directory for storing cached data. + + When running Mopidy as a regular user, this should usually be + ``$XDG_CACHE_DIR/mopidy``, i.e. :file:`~/.cache/mopidy`. + + When running Mopidy as a system service, this should usually be + :file:`/var/cache/mopidy`. .. confval:: core/config_dir - Path to directory with configuration files. + Path to base directory for config files. + + When running Mopidy as a regular user, this should usually be + ``$XDG_CONFIG_DIR/mopidy``, i.e. :file:`~/.config/mopidy`. + + When running Mopidy as a system service, this should usually be + :file:`/etc/mopidy`. .. confval:: core/data_dir - Path to directory with data files. + Path to base directory for persistent data files. + + When running Mopidy as a regular user, this should usually be + ``$XDG_DATA_DIR/mopidy``, i.e. :file:`~/.local/share/mopidy`. + + When running Mopidy as a system service, this should usually be + :file:`/var/lib/mopidy`. .. confval:: core/max_tracklist_length diff --git a/mopidy/ext.py b/mopidy/ext.py index cdaa5662..908b6d5d 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -61,9 +61,9 @@ class Extension(object): return schema def get_cache_dir(self, config): - """Get or create cache directory for extension and return path + """Get or create cache directory for the extension. - :param config: Loaded configuration + :param config: the Mopidy config object :return: string """ assert self.ext_name is not None @@ -73,9 +73,9 @@ class Extension(object): return cache_dir_path def get_config_dir(self, config): - """Get or create configuration directory for extension and return path + """Get or create configuration directory for the extension. - :param config: Loaded configuration + :param config: the Mopidy config object :return: string """ assert self.ext_name is not None @@ -85,9 +85,9 @@ class Extension(object): return config_dir_path def get_data_dir(self, config): - """Get or create data directory for extension and return path + """Get or create data directory for the extension. - :param config: Loaded configuration + :param config: the Mopidy config object :returns: string """ assert self.ext_name is not None From 81063995986f8efe85de2e05f093ce78d83b00e3 Mon Sep 17 00:00:00 2001 From: Mark Greenwood Date: Sun, 26 Jul 2015 12:55:46 +0100 Subject: [PATCH 38/94] Simplify the whole thing by using taglist types and not bothering with the config option or command to switch it on --- mopidy/mpd/protocol/tagtype_list.py | 2 ++ mopidy/mpd/translator.py | 6 ++++++ tests/mpd/test_translator.py | 7 +++++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/mopidy/mpd/protocol/tagtype_list.py b/mopidy/mpd/protocol/tagtype_list.py index 575b1aaf..d9dee145 100644 --- a/mopidy/mpd/protocol/tagtype_list.py +++ b/mopidy/mpd/protocol/tagtype_list.py @@ -19,4 +19,6 @@ TAGTYPE_LIST = [ 'MUSICBRAINZ_ALBUMID', 'MUSICBRAINZ_ALBUMARTISTID', 'MUSICBRAINZ_TRACKID', + 'X-AlbumUri', + 'X-AlbumImage', ] diff --git a/mopidy/mpd/translator.py b/mopidy/mpd/translator.py index ef16ed28..4aa4bdb9 100644 --- a/mopidy/mpd/translator.py +++ b/mopidy/mpd/translator.py @@ -97,6 +97,12 @@ def track_to_mpd_format(track, position=None, stream_title=None): if track.musicbrainz_id is not None: result.append(('MUSICBRAINZ_TRACKID', track.musicbrainz_id)) + if track.album and track.album.uri: + result.append(('X-AlbumUri', track.album.uri)) + if track.album and track.album.images: + images = ';'.join(i for i in track.album.images if i is not '') + result.append(('X-AlbumImage', images)) + result = [element for element in result if _has_value(*element)] return result diff --git a/tests/mpd/test_translator.py b/tests/mpd/test_translator.py index 57477a51..6a3767af 100644 --- a/tests/mpd/test_translator.py +++ b/tests/mpd/test_translator.py @@ -14,7 +14,8 @@ class TrackMpdFormatTest(unittest.TestCase): name='a name', album=Album( name='an album', num_tracks=13, - artists=[Artist(name='an other artist')]), + artists=[Artist(name='an other artist')], + uri='urischeme:album:12345', images=['image1', 'image2']), track_no=7, composers=[Artist(name='a composer')], performers=[Artist(name='a performer')], @@ -76,8 +77,10 @@ class TrackMpdFormatTest(unittest.TestCase): self.assertIn(('Disc', 1), result) self.assertIn(('Pos', 9), result) self.assertIn(('Id', 122), result) + self.assertIn(('X-AlbumUri', 'urischeme:album:12345'), result) + self.assertIn(('X-AlbumImage', 'image2;image1'), result) self.assertNotIn(('Comment', 'a comment'), result) - self.assertEqual(len(result), 14) + self.assertEqual(len(result), 16) def test_track_to_mpd_format_with_last_modified(self): track = self.track.replace(last_modified=995303899000) From 310fcdf0ef680fbdef09b311a4a88e87d02aa63e Mon Sep 17 00:00:00 2001 From: Mark Greenwood Date: Sun, 26 Jul 2015 13:04:33 +0100 Subject: [PATCH 39/94] Unexpected test failure fixed --- tests/mpd/test_translator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mpd/test_translator.py b/tests/mpd/test_translator.py index 6a3767af..dc58bac2 100644 --- a/tests/mpd/test_translator.py +++ b/tests/mpd/test_translator.py @@ -78,7 +78,7 @@ class TrackMpdFormatTest(unittest.TestCase): self.assertIn(('Pos', 9), result) self.assertIn(('Id', 122), result) self.assertIn(('X-AlbumUri', 'urischeme:album:12345'), result) - self.assertIn(('X-AlbumImage', 'image2;image1'), result) + self.assertIn(('X-AlbumImage', 'image1;image2'), result) self.assertNotIn(('Comment', 'a comment'), result) self.assertEqual(len(result), 16) From c6a831b40a0ea2798dee08618995bd0645c31415 Mon Sep 17 00:00:00 2001 From: Mark Greenwood Date: Sun, 26 Jul 2015 13:18:30 +0100 Subject: [PATCH 40/94] Update changelog for protocol extensions --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index fb587c6c..71ae42cf 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -116,6 +116,9 @@ MPD frontend - Exclude empty tags fields from metadata output. (Fixes: :issue:`1045`, PR: :issue:`1235`) +- Implement protocol extensions to output Album URIs and Album Images when + outputting track data to clients. (PR: :issue:`1230`) + Stream backend -------------- From 2b06b778659c640edd0ad69df78cfc056ede54b3 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 26 Jul 2015 14:51:26 +0200 Subject: [PATCH 41/94] docs: Update authors --- AUTHORS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/AUTHORS b/AUTHORS index c2baca6c..13648195 100644 --- a/AUTHORS +++ b/AUTHORS @@ -61,4 +61,7 @@ - Tom Roth - Mark Greenwood - Stein Karlsen +- Dejan Prokić - Eric Jahn +- Mikhail Golubev +- Danilo Bargen From 9649dc07774ed35a7e0c0da5d8e37b2db8e33e7a Mon Sep 17 00:00:00 2001 From: Mark Greenwood Date: Sun, 26 Jul 2015 13:57:46 +0100 Subject: [PATCH 42/94] list order in python is not deterministic, hence had to change the test --- tests/mpd/test_translator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mpd/test_translator.py b/tests/mpd/test_translator.py index dc58bac2..65c80bbb 100644 --- a/tests/mpd/test_translator.py +++ b/tests/mpd/test_translator.py @@ -15,7 +15,7 @@ class TrackMpdFormatTest(unittest.TestCase): album=Album( name='an album', num_tracks=13, artists=[Artist(name='an other artist')], - uri='urischeme:album:12345', images=['image1', 'image2']), + uri='urischeme:album:12345', images=['image1']), track_no=7, composers=[Artist(name='a composer')], performers=[Artist(name='a performer')], @@ -78,7 +78,7 @@ class TrackMpdFormatTest(unittest.TestCase): self.assertIn(('Pos', 9), result) self.assertIn(('Id', 122), result) self.assertIn(('X-AlbumUri', 'urischeme:album:12345'), result) - self.assertIn(('X-AlbumImage', 'image1;image2'), result) + self.assertIn(('X-AlbumImage', 'image1'), result) self.assertNotIn(('Comment', 'a comment'), result) self.assertEqual(len(result), 16) From 3810089be39a4301cad31445ba4bdab84dbc3375 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 27 Jul 2015 13:41:29 +0200 Subject: [PATCH 43/94] tests: Avoid import errors during conftest setup --- tests/file/conftest.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/file/conftest.py b/tests/file/conftest.py index c93a71f2..c4ba96f4 100644 --- a/tests/file/conftest.py +++ b/tests/file/conftest.py @@ -2,8 +2,6 @@ from __future__ import unicode_literals import pytest -from mopidy.file import library - @pytest.fixture def file_config(): @@ -15,4 +13,8 @@ def file_config(): @pytest.fixture def file_library(file_config): + # Import library, thus scanner, thus gobject as late as possible to avoid + # hard to track import errors during conftest setup. + from mopidy.file import library + return library.FileLibraryProvider(backend=None, config=file_config) From 1eb41aca7d47828493e5671a32e37d82ec9e0b37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rnar=20Snoksrud?= Date: Mon, 27 Jul 2015 02:02:19 +0200 Subject: [PATCH 44/94] tests: fix test breakage due to promotion from int to long This fixes #1240. In internals/path.py. there is a snippet of code that multiples mtime for a file with 1000, and then casting it to `int`, to return the number of milliseconds since epoch (or whatever). This will, however, not ensure that the result is an `int`. >>> type(int(2**32)) Instead, fix the tests to look for (int, long), and clarify the implementation. This bug found on a 32-bit VM :) --- mopidy/internal/path.py | 3 ++- tests/__init__.py | 2 +- tests/core/test_history.py | 2 +- tests/internal/test_path.py | 12 ++++++++++++ 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/mopidy/internal/path.py b/mopidy/internal/path.py index f56520f0..8c560187 100644 --- a/mopidy/internal/path.py +++ b/mopidy/internal/path.py @@ -192,7 +192,8 @@ def _find(root, thread_count=10, relative=False, follow=False): def find_mtimes(root, follow=False): results, errors = _find(root, relative=False, follow=follow) - mtimes = dict((f, int(st.st_mtime * 1000)) for f, st in results.items()) + # return the mtimes as integer milliseconds + mtimes = {f: int(st.st_mtime * 1000) for f, st in results.items()} return mtimes, errors diff --git a/tests/__init__.py b/tests/__init__.py index fc8d5dcf..c76c48f0 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -32,6 +32,6 @@ class IsA(object): return str(self.klass) -any_int = IsA(int) +any_int = IsA((int, long)) any_str = IsA(str) any_unicode = IsA(compat.text_type) diff --git a/tests/core/test_history.py b/tests/core/test_history.py index 48062aaf..068518b6 100644 --- a/tests/core/test_history.py +++ b/tests/core/test_history.py @@ -40,7 +40,7 @@ class PlaybackHistoryTest(unittest.TestCase): result = self.history.get_history() (timestamp, ref) = result[0] - self.assertIsInstance(timestamp, int) + self.assertIsInstance(timestamp, (int, long)) self.assertEqual(track.uri, ref.uri) self.assertIn(track.name, ref.name) for artist in track.artists: diff --git a/tests/internal/test_path.py b/tests/internal/test_path.py index 503d2490..0d266725 100644 --- a/tests/internal/test_path.py +++ b/tests/internal/test_path.py @@ -380,6 +380,18 @@ class FindMTimesTest(unittest.TestCase): self.assertEqual(expected, result) self.assertEqual({}, errors) + def test_gives_mtime_in_milliseconds(self): + fname = self.touch('foobar') + + os.utime(fname, (1, 3.14159265)) + + result, errors = path.find_mtimes(fname) + + self.assertEqual(len(result), 1) + mtime, = result.values() + self.assertEqual(mtime, 3141) + self.assertEqual(errors, {}) + # TODO: kill this in favour of just os.path.getmtime + mocks class MtimeTest(unittest.TestCase): From c8d31e94b7647fd76667dd48e37973c2f8d36bd8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 6 Aug 2015 01:00:00 +0200 Subject: [PATCH 45/94] mpd: Faster playlist listing --- docs/changelog.rst | 8 ++++ mopidy/mpd/protocol/stored_playlists.py | 16 ++++---- tests/mpd/protocol/test_music_db.py | 43 +++++++++++++-------- tests/mpd/protocol/test_stored_playlists.py | 12 ++++-- 4 files changed, 51 insertions(+), 28 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 71ae42cf..651b22ba 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -119,6 +119,14 @@ MPD frontend - Implement protocol extensions to output Album URIs and Album Images when outputting track data to clients. (PR: :issue:`1230`) +- The MPD commands ``lsinfo`` and ``listplaylists`` are now implemented using + the :meth:`~mopidy.core.PlaylistsProvider.as_list` method, which retrieves a + lot less data and is thus much faster than the deprecated + :meth:`~mopidy.core.PlaylistsProvider.get_playlists`. The drawback is that + the ``Last-Modified`` timestamp is not available through this method, and the + timestamps in the MPD command responses are now always set to the current + time. + Stream backend -------------- diff --git a/mopidy/mpd/protocol/stored_playlists.py b/mopidy/mpd/protocol/stored_playlists.py index a5d4b180..bf31fa10 100644 --- a/mopidy/mpd/protocol/stored_playlists.py +++ b/mopidy/mpd/protocol/stored_playlists.py @@ -75,29 +75,29 @@ def listplaylists(context): - ncmpcpp 0.5.10 segfaults if we return 'playlist: ' on a line, so we must ignore playlists without names, which isn't very useful anyway. """ + last_modified = _get_last_modified() result = [] - for playlist in context.core.playlists.get_playlists().get(): - if not playlist.name: + for playlist_ref in context.core.playlists.as_list().get(): + if not playlist_ref.name: continue - name = context.lookup_playlist_name_from_uri(playlist.uri) + name = context.lookup_playlist_name_from_uri(playlist_ref.uri) result.append(('playlist', name)) - result.append(('Last-Modified', _get_last_modified(playlist))) + result.append(('Last-Modified', last_modified)) return result # TODO: move to translators? -def _get_last_modified(playlist): +def _get_last_modified(last_modified=None): """Formats last modified timestamp of a playlist for MPD. Time in UTC with second precision, formatted in the ISO 8601 format, with the "Z" time zone marker for UTC. For example, "1970-01-01T00:00:00Z". """ - if playlist.last_modified is None: + if last_modified is None: # If unknown, assume the playlist is modified dt = datetime.datetime.utcnow() else: - dt = datetime.datetime.utcfromtimestamp( - playlist.last_modified / 1000.0) + dt = datetime.datetime.utcfromtimestamp(last_modified / 1000.0) dt = dt.replace(microsecond=0) return '%sZ' % dt.isoformat() diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index 5fe40e0d..ea944b7a 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -2,8 +2,10 @@ from __future__ import absolute_import, unicode_literals import unittest +import mock + from mopidy.models import Album, Artist, Playlist, Ref, SearchResult, Track -from mopidy.mpd.protocol import music_db +from mopidy.mpd.protocol import music_db, stored_playlists from tests.mpd import protocol @@ -299,33 +301,37 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.send_request('listfiles') self.assertEqualResponse('ACK [0@0] {listfiles} Not implemented') - def test_lsinfo_without_path_returns_same_as_for_root(self): - last_modified = 1390942873222 + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_without_path_returns_same_as_for_root( + self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:/a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:/a')]) response1 = self.send_request('lsinfo') response2 = self.send_request('lsinfo "/"') self.assertEqual(response1, response2) - def test_lsinfo_with_empty_path_returns_same_as_for_root(self): - last_modified = 1390942873222 + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_with_empty_path_returns_same_as_for_root( + self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:/a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:/a')]) response1 = self.send_request('lsinfo ""') response2 = self.send_request('lsinfo "/"') self.assertEqual(response1, response2) - def test_lsinfo_for_root_includes_playlists(self): - last_modified = 1390942873222 + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_for_root_includes_playlists(self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:/a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:/a')]) self.send_request('lsinfo "/"') self.assertInResponse('playlist: a') - # Date without milliseconds and with time zone information - self.assertInResponse('Last-Modified: 2014-01-28T21:01:13Z') + self.assertInResponse('Last-Modified: 2015-08-05T22:51:06Z') self.assertInResponse('OK') def test_lsinfo_for_root_includes_dirs_for_each_lib_with_content(self): @@ -337,7 +343,10 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertInResponse('directory: dummy') self.assertInResponse('OK') - def test_lsinfo_for_dir_with_and_without_leading_slash_is_the_same(self): + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_for_dir_with_and_without_leading_slash_is_the_same( + self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.library.dummy_browse_result = { 'dummy:/': [Ref.track(uri='dummy:/a', name='a'), Ref.directory(uri='dummy:/foo', name='foo')]} @@ -346,7 +355,10 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): response2 = self.send_request('lsinfo "/dummy"') self.assertEqual(response1, response2) - def test_lsinfo_for_dir_with_and_without_trailing_slash_is_the_same(self): + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_for_dir_with_and_without_trailing_slash_is_the_same( + self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.library.dummy_browse_result = { 'dummy:/': [Ref.track(uri='dummy:/a', name='a'), Ref.directory(uri='dummy:/foo', name='foo')]} @@ -404,12 +416,11 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_lsinfo_for_root_returns_browse_result_before_playlists(self): - last_modified = 1390942873222 self.backend.library.dummy_browse_result = { 'dummy:/': [Ref.track(uri='dummy:/a', name='a'), Ref.directory(uri='dummy:/foo', name='foo')]} self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:/a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:/a')]) response = self.send_request('lsinfo "/"') self.assertLess(response.index('directory: dummy'), diff --git a/tests/mpd/protocol/test_stored_playlists.py b/tests/mpd/protocol/test_stored_playlists.py index 2899a126..90c325ff 100644 --- a/tests/mpd/protocol/test_stored_playlists.py +++ b/tests/mpd/protocol/test_stored_playlists.py @@ -1,6 +1,9 @@ from __future__ import absolute_import, unicode_literals +import mock + from mopidy.models import Playlist, Track +from mopidy.mpd.protocol import stored_playlists from tests.mpd import protocol @@ -76,15 +79,16 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertNotInResponse('Pos: 0') self.assertInResponse('OK') - def test_listplaylists(self): - last_modified = 1390942873222 + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_listplaylists(self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:a')]) self.send_request('listplaylists') self.assertInResponse('playlist: a') # Date without milliseconds and with time zone information - self.assertInResponse('Last-Modified: 2014-01-28T21:01:13Z') + self.assertInResponse('Last-Modified: 2015-08-05T22:51:06Z') self.assertInResponse('OK') def test_listplaylists_duplicate(self): From f3fd25896277ccf239bd5f8aa1c85397c9503f18 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 6 Aug 2015 07:55:59 +0200 Subject: [PATCH 46/94] docs: Update authors --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 13648195..6a911e7b 100644 --- a/AUTHORS +++ b/AUTHORS @@ -65,3 +65,4 @@ - Eric Jahn - Mikhail Golubev - Danilo Bargen +- Bjørnar Snoksrud From 69dc639ab350a9eae08c7645d55ac1a619ec6b1f Mon Sep 17 00:00:00 2001 From: Giorgos Logiotatidis Date: Fri, 29 May 2015 20:25:27 +0300 Subject: [PATCH 47/94] Support loading of m3u8 playlists. Unicode is nice, let's support it! --- mopidy/m3u/playlists.py | 2 +- mopidy/m3u/translator.py | 12 ++++++++---- tests/m3u/test_translator.py | 13 +++++++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index 33281129..e96c0412 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -63,7 +63,7 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): playlists = {} encoding = sys.getfilesystemencoding() - for path in glob.glob(os.path.join(self._playlists_dir, b'*.m3u')): + for path in glob.glob(os.path.join(self._playlists_dir, b'*.m3u*')): relpath = os.path.basename(path) uri = translator.path_to_playlist_uri(relpath) name = os.path.splitext(relpath)[0].decode(encoding, 'replace') diff --git a/mopidy/m3u/translator.py b/mopidy/m3u/translator.py index a6e006b1..908e218f 100644 --- a/mopidy/m3u/translator.py +++ b/mopidy/m3u/translator.py @@ -74,11 +74,14 @@ def parse_m3u(file_path, media_dir=None): - Lines starting with # are ignored, except for extended M3U directives. - Track.name and Track.length are set from extended M3U directives. - m3u files are latin-1. + - m3u8 files are utf-8 """ # TODO: uris as bytes + file_encoding = 'utf-8' if file_path.endswith(b'.m3u8') else 'latin1' + tracks = [] try: - with open(file_path) as m3u: + with codecs.open(file_path, 'rb', file_encoding, 'replace') as m3u: contents = m3u.readlines() except IOError as error: logger.warning('Couldn\'t open m3u: %s', locale_decode(error)) @@ -87,12 +90,13 @@ def parse_m3u(file_path, media_dir=None): if not contents: return tracks - extended = contents[0].decode('latin1').startswith('#EXTM3U') + # Strip newlines left by codecs + contents = [line.strip() for line in contents] + + extended = contents[0].startswith('#EXTM3U') track = Track() for line in contents: - line = line.strip().decode('latin1') - if line.startswith('#'): if extended and line.startswith('#EXTINF'): track = m3u_extinf_to_track(line) diff --git a/tests/m3u/test_translator.py b/tests/m3u/test_translator.py index fc7fc958..bacb589f 100644 --- a/tests/m3u/test_translator.py +++ b/tests/m3u/test_translator.py @@ -15,12 +15,15 @@ from tests import path_to_data_dir data_dir = path_to_data_dir('') song1_path = path_to_data_dir('song1.mp3') song2_path = path_to_data_dir('song2.mp3') +song3_path = path_to_data_dir('φοο.mp3') encoded_path = path_to_data_dir('æøå.mp3') song1_uri = path.path_to_uri(song1_path) song2_uri = path.path_to_uri(song2_path) +song3_uri = path.path_to_uri(song3_path) encoded_uri = path.path_to_uri(encoded_path) song1_track = Track(uri=song1_uri) song2_track = Track(uri=song2_uri) +song3_track = Track(uri=song3_uri) encoded_track = Track(uri=encoded_uri) song1_ext_track = song1_track.copy(name='song1') song2_ext_track = song2_track.copy(name='song2', length=60000) @@ -115,6 +118,16 @@ class M3UToUriTest(unittest.TestCase): tracks = self.parse(path_to_data_dir('encoding-ext.m3u')) self.assertEqual([encoded_ext_track], tracks) + def test_m3u8_file(self): + with tempfile.NamedTemporaryFile(suffix='.m3u8', delete=False) as tmp: + tmp.write(song3_path) + try: + tracks = self.parse(tmp.name) + self.assertEqual([song3_track], tracks) + finally: + if os.path.exists(tmp.name): + os.remove(tmp.name) + class URItoM3UTest(unittest.TestCase): pass From 4ab1a2445f15cb681a0f554e8ec5db2ef16bd218 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 7 Aug 2015 15:31:26 +0200 Subject: [PATCH 48/94] docs: Add m3u8 loading support to changelog --- docs/changelog.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 651b22ba..27b634bf 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -164,6 +164,12 @@ has a well defined and limited scope, while splitting the more feature rich Mopidy-Local extension out to an independent project. (Fixes: :issue:`1004`, PR: :issue:`1207`) +M3U backend +----------- + +- Support loading UTF-8 encoded M3U files with the ``.m3u8`` file extension. + (PR: :issue:`1193`) + Utils ----- From a16a2c5a1b0c1b5dc919578402383d41802faa6b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 7 Aug 2015 15:34:28 +0200 Subject: [PATCH 49/94] tests: Fix test made flaky by lsinfo Last-Modified change --- tests/mpd/protocol/test_regression.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/mpd/protocol/test_regression.py b/tests/mpd/protocol/test_regression.py index ff0141f2..565b369e 100644 --- a/tests/mpd/protocol/test_regression.py +++ b/tests/mpd/protocol/test_regression.py @@ -2,7 +2,10 @@ from __future__ import absolute_import, unicode_literals import random +import mock + from mopidy.models import Playlist, Ref, Track +from mopidy.mpd.protocol import stored_playlists from tests.mpd import protocol @@ -214,12 +217,14 @@ class IssueGH1120RegressionTest(protocol.BaseTestCase): """ - def test(self): + @mock.patch.object(stored_playlists, '_get_last_modified') + def test(self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.library.dummy_browse_result = { 'dummy:/': [Ref.playlist(name='Top 100 tracks', uri='dummy:/1')], } self.backend.playlists.set_dummy_playlists([ - Playlist(name='Top 100 tracks', uri='dummy:/1', last_modified=123), + Playlist(name='Top 100 tracks', uri='dummy:/1'), ]) response1 = self.send_request('lsinfo "/"') From 645071c61f2070fdcbce3dcad1d3dbdf7ba2c763 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 9 Aug 2015 22:13:16 +0200 Subject: [PATCH 50/94] docs: Update authors --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 6a911e7b..a370ce6c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -66,3 +66,4 @@ - Mikhail Golubev - Danilo Bargen - Bjørnar Snoksrud +- Giorgos Logiotatidis From 21f941c323eb50f224910f740763c296fd6f6e22 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 9 Aug 2015 22:40:54 +0200 Subject: [PATCH 51/94] models: Add Artist.sortname to init docstring --- mopidy/models/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mopidy/models/__init__.py b/mopidy/models/__init__.py index c84254cd..7afa2db8 100644 --- a/mopidy/models/__init__.py +++ b/mopidy/models/__init__.py @@ -107,6 +107,8 @@ class Artist(ValidatedImmutableObject): :type uri: string :param name: artist name :type name: string + :param sortname: artist name for sorting + :type sortname: string :param musicbrainz_id: MusicBrainz ID :type musicbrainz_id: string """ From f3e61daa9232b7274ad841c32e924f889a523c39 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 9 Aug 2015 22:39:42 +0200 Subject: [PATCH 52/94] docs: Update changelog --- docs/changelog.rst | 166 ++++++++++++++++++++++++++------------------- 1 file changed, 97 insertions(+), 69 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 27b634bf..1c5f5332 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,21 +7,52 @@ This changelog is used to track all major changes to Mopidy. v1.1.0 (UNRELEASED) =================== +Mopidy 1.1 is here! + +Since the release of 1.0, we've closed or merged approximately 55 issues and +pull requests through about 400 commits by a record high 20 extraordinary +people, including 14 newcomers. That's less issues and commits than in the 1.0 +release, but even more contributors, and a doubling of the number of newcomers. +Thanks to :ref:`everyone ` who has :ref:`contributed `! + +As we promised with the release of Mopidy 1.0, any extension working with +Mopidy 1.0 should continue working with all Mopidy 1.x releases. However, this +release brings a lot stronger enforcement of our documented APIs. If an +extension doesn't use the APIs properly, it may no longer work. The advantage +of this change is that Mopidy is now more robust against errors in extensions, +and also provides vastly better error messages when extensions misbehave. This +should make it easier to create quality extensions. + +The major features of Mopidy 1.1 are: + +- Validation of the arguments to all core API methods, as well as all responses + from backends and all data model attributes. + +- New bundled backend, Mopidy-File. It is similar to Mopidy-Local, but allows + you to browse and play music from local disk without running a scan to index + the music first. The drawback is that it doesn't support searching. + +- The Mopidy-MPD server should now be up to date with the 0.19 version of the + MPD protocol. + Dependencies ------------ - Mopidy now requires Requests. +- Heads up: Porting from GStreamer 0.10 to 1.x and support for running Mopidy + with Python 3.4+ is not far off on our roadmap. + Core API -------- -- Calling the following methods with ``kwargs`` is being deprecated. - (PR: :issue:`1090`) +- **Deprecated:** Calling the following methods with ``kwargs`` is being + deprecated. (PR: :issue:`1090`) - - :meth:`mopidy.core.library.LibraryController.search` - - :meth:`mopidy.core.library.PlaylistsController.filter` - - :meth:`mopidy.core.library.TracklistController.filter` - - :meth:`mopidy.core.library.TracklistController.remove` + - :meth:`mopidy.core.LibraryController.search` + - :meth:`mopidy.core.PlaylistsController.filter` + - :meth:`mopidy.core.TracklistController.filter` + - :meth:`mopidy.core.TracklistController.remove` - Updated core controllers to handle backend exceptions in all calls that rely on multiple backends. (Issue: :issue:`667`) @@ -33,13 +64,13 @@ Core API ``tl_track`` versions of the calls. (Fixes: :issue:`1131` PR: :issue:`1136`, :issue:`1140`) -- Add :meth:`mopidy.core.playback.PlaybackController.get_current_tlid`. +- Add :meth:`mopidy.core.PlaybackController.get_current_tlid`. (Part of: :issue:`1137`) - Update core to handle backend crashes and bad data. (Fixes: :issue:`1161`) -- Add `max_tracklist_length` config and limitation. (Fixes: :issue:`997` - PR: :issue:`1225`) +- Add :confval:`core/max_tracklist_length` config and limitation. (Fixes: + :issue:`997` PR: :issue:`1225`) - Added ``playlist_deleted`` event. (Fixes: :issue:`996`) @@ -52,7 +83,7 @@ Models - Memory usage for models has been greatly improved. We now have a lower overhead per instance by using slots, intern identifiers and automatically reuse instances. For the test data set this was developed against, a library - of ~14000 tracks, went from needing ~75MB to ~17MB. (Fixes: :issue:`348`) + of ~14.000 tracks, went from needing ~75MB to ~17MB. (Fixes: :issue:`348`) - Added :attr:`mopidy.models.Artist.sortname` field that is mapped to ``musicbrainz-sortname`` tag. (Fixes: :issue:`940`) @@ -68,7 +99,7 @@ Extension support ----------------- - Add new methods to :class:`~mopidy.ext.Extension` class for getting cache, - config and data dirs specific to your extension: + config and data directories specific to your extension: - :meth:`mopidy.ext.Extension.get_cache_dir` - :meth:`mopidy.ext.Extension.get_config_dir` @@ -78,6 +109,58 @@ Extension support both when Mopidy is run by a regular user and when run as a system service. (Fixes: :issue:`843`, PR: :issue:`1232`) +- Add :func:`mopidy.httpclient.format_proxy` and + :func:`mopidy.httpclient.format_user_agent`. (Part of: :issue:`1156`) + +- It is now possible to import :mod:`mopidy.backends` without having GObject or + GStreamer installed. In other words, a lot of backend extensions should now + be able to run tests in a virtualenv with global site-packages disabled. This + removes a lot of potential error sources. (Fixes: :issue:`1068`, PR: + :issue:`1115`) + +Local backend +------------- + +- Filter out :class:`None` from + :meth:`~mopidy.backend.LibraryProvider.get_distinct` results. All returned + results should be strings. (Fixes: :issue:`1202`) + +Stream backend +-------------- + +- Move stream playlist parsing from GStreamer to the stream backend. (Fixes: + :issue:`671`) + +File backend +------------ + +The :ref:`Mopidy-File ` backend is a new bundled backend. It is +similar to Mopidy-Local since it works with local files, but it differs in a +few key ways: + +- Mopidy-File lets you browse your media files by their file hierarchy. + +- It supports multiple media directories, all exposed under the "Files" + directory when you browse your library with e.g. an MPD client. + +- There is no index of the media files, like the JSON or SQLite files used by + Mopidy-Local. Thus no need to scan the music collection before starting + Mopidy. Everything is read from the file system when needed and changes to + the file system is thus immediately visible in Mopidy clients. + +- Because there is no index, there is no support for search. + +Our long term plan is to keep this very simple file backend in Mopidy, as it +has a well defined and limited scope, while splitting the more feature rich +Mopidy-Local extension out to an independent project. (Fixes: :issue:`1004`, +PR: :issue:`1207`) + +M3U backend +----------- + +- Support loading UTF-8 encoded M3U files with the ``.m3u8`` file extension. + (PR: :issue:`1193`) + MPD frontend ------------ @@ -120,74 +203,19 @@ MPD frontend outputting track data to clients. (PR: :issue:`1230`) - The MPD commands ``lsinfo`` and ``listplaylists`` are now implemented using - the :meth:`~mopidy.core.PlaylistsProvider.as_list` method, which retrieves a - lot less data and is thus much faster than the deprecated - :meth:`~mopidy.core.PlaylistsProvider.get_playlists`. The drawback is that + the :meth:`~mopidy.core.PlaylistsController.as_list` method, which retrieves + a lot less data and is thus much faster than the deprecated + :meth:`~mopidy.core.PlaylistsController.get_playlists`. The drawback is that the ``Last-Modified`` timestamp is not available through this method, and the timestamps in the MPD command responses are now always set to the current time. -Stream backend --------------- - -- Move stream playlist parsing from GStreamer to the stream backend. (Fixes: - :issue:`671`) - -Local backend -------------- - -- Filter out :class:`None` from - :meth:`~mopidy.backend.LibraryProvider.get_distinct` results. All returned - results should be strings. (Fixes: :issue:`1202`) - -File backend ------------- - -The :ref:`Mopidy-File ` backend is a new bundled backend. It is -similar to Mopidy-Local since it works with local files, but it differs in a -few key ways: - -- Mopidy-File lets you browse your media files by their file hierarchy. - -- It supports multiple media directories, all exposed under the "Files" - directory when you browse your library with e.g. an MPD client. - -- There is no index of the media files, like the JSON or SQLite files used by - Mopidy-Local. Thus no need to scan the music collection before starting - Mopidy. Everything is read from the file system when needed and changes to - the file system is thus immediately visible in Mopidy clients. - -- Because there is no index, there is no support for search. - -Our long term plan is to keep this very simple file backend in Mopidy, as it -has a well defined and limited scope, while splitting the more feature rich -Mopidy-Local extension out to an independent project. (Fixes: :issue:`1004`, -PR: :issue:`1207`) - -M3U backend ------------ - -- Support loading UTF-8 encoded M3U files with the ``.m3u8`` file extension. - (PR: :issue:`1193`) - -Utils ------ - -- Add :func:`mopidy.httpclient.format_proxy` and - :func:`mopidy.httpclient.format_user_agent`. (Part of: :issue:`1156`) - Internal changes ---------------- - Tests have been cleaned up to stop using deprecated APIs where feasible. (Partial fix: :issue:`1083`, PR: :issue:`1090`) -- It is now possible to import :mod:`mopidy.backends` without having GObject or - GStreamer installed. In other words, a lot of backend extensions should now - be able to run tests in a virtualenv with global site-packages disabled. This - removes a lot of potential error sources. (Fixes: :issue:`1068`, PR: - :issue:`1115`) - v1.0.8 (2015-07-22) =================== From 412631291ffafdb8901b1e7f88a8829024555076 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 9 Aug 2015 23:38:49 +0200 Subject: [PATCH 53/94] core: Tweak deprecation wording I know "not supported" and "deprecated" can mean the same, but I usually read the following meaning into it: - deprecation ~= don't use this, it will stop working in the future - not supported ~= don't use this, it already stopped working Thus I try to only use the "deprecated" term. --- mopidy/core/tracklist.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 1938f001..db4e2a69 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -554,7 +554,7 @@ class TracklistController(object): :rtype: list of :class:`mopidy.models.TlTrack` that was removed .. deprecated:: 1.1 - Providing the criteria via ``kwargs`` is no longer supported. + Providing the criteria via ``kwargs``. """ if kwargs: deprecation.warn('core.tracklist.remove:kwargs_criteria') From e8303c9402fb6f0c828f3891a9d2a5e731e515ad Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 9 Aug 2015 23:42:35 +0200 Subject: [PATCH 54/94] docs: intern -> interned --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 1c5f5332..5e89af53 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -81,7 +81,7 @@ Models serialization. (Fixes: :issue:`865`) - Memory usage for models has been greatly improved. We now have a lower - overhead per instance by using slots, intern identifiers and automatically + overhead per instance by using slots, interned identifiers and automatically reuse instances. For the test data set this was developed against, a library of ~14.000 tracks, went from needing ~75MB to ~17MB. (Fixes: :issue:`348`) From 551959594727edea45eac2a494559aa68efee249 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 9 Aug 2015 23:43:07 +0200 Subject: [PATCH 55/94] docs: Add v1.1.0 release date --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5e89af53..1bc24394 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,7 +4,7 @@ Changelog This changelog is used to track all major changes to Mopidy. -v1.1.0 (UNRELEASED) +v1.1.0 (2015-08-09) =================== Mopidy 1.1 is here! From 83010813a193a52d28d188ad70d1bad16aaae0a2 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 9 Aug 2015 23:43:42 +0200 Subject: [PATCH 56/94] Bump version to 1.1.0 --- 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 4752f080..40308a53 100644 --- a/mopidy/__init__.py +++ b/mopidy/__init__.py @@ -14,4 +14,4 @@ if not (2, 7) <= sys.version_info < (3,): warnings.filterwarnings('ignore', 'could not open display') -__version__ = '1.0.8' +__version__ = '1.1.0' diff --git a/tests/test_version.py b/tests/test_version.py index e914efc4..011c8de7 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -64,5 +64,6 @@ class VersionTest(unittest.TestCase): self.assertVersionLess('1.0.4', '1.0.5') self.assertVersionLess('1.0.5', '1.0.6') self.assertVersionLess('1.0.6', '1.0.7') - self.assertVersionLess('1.0.7', __version__) - self.assertVersionLess(__version__, '1.0.9') + self.assertVersionLess('1.0.7', '1.0.8') + self.assertVersionLess('1.0.8', __version__) + self.assertVersionLess(__version__, '1.1.1') From 784c9758ab0acf84cf302cb99dc6dd07f4b779d8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 10 Aug 2015 00:35:33 +0200 Subject: [PATCH 57/94] docs: Include issues/PRs fixed at sprint --- docs/changelog.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 1bc24394..75fdc538 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,11 +9,13 @@ v1.1.0 (2015-08-09) Mopidy 1.1 is here! -Since the release of 1.0, we've closed or merged approximately 55 issues and +Since the release of 1.0, we've closed or merged approximately 65 issues and pull requests through about 400 commits by a record high 20 extraordinary people, including 14 newcomers. That's less issues and commits than in the 1.0 release, but even more contributors, and a doubling of the number of newcomers. -Thanks to :ref:`everyone ` who has :ref:`contributed `! +Thanks to :ref:`everyone ` who has :ref:`contributed `, +especially those that joined the sprint at EuroPython 2015 in Bilbao, Spain a +couple of weeks ago! As we promised with the release of Mopidy 1.0, any extension working with Mopidy 1.0 should continue working with all Mopidy 1.x releases. However, this From 0f4771f5398be96d863c2a0c9e9d3b9fd85ab9ad Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 11 Aug 2015 10:02:46 +0200 Subject: [PATCH 58/94] file: Add missing space to log message --- mopidy/file/library.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/file/library.py b/mopidy/file/library.py index 10586561..691020b4 100644 --- a/mopidy/file/library.py +++ b/mopidy/file/library.py @@ -108,7 +108,7 @@ class FileLibraryProvider(backend.LibraryProvider): media_dir_split[0].encode(FS_ENCODING)) if not local_path: - logger.warning('Failed expanding path (%s) from' + logger.warning('Failed expanding path (%s) from ' 'file/media_dirs config value.', media_dir_split[0]) continue From 326f8579ca489998f7ef26a2064b69e43c3bc986 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 11 Aug 2015 10:03:33 +0200 Subject: [PATCH 59/94] core: Add quotes around URI scheme in log message --- mopidy/core/actor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index f20b0ba2..6d10a593 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -161,7 +161,7 @@ class Backends(list): for scheme in b.uri_schemes.get(): assert scheme not in backends_by_scheme, ( - 'Cannot add URI scheme %s for %s, ' + 'Cannot add URI scheme "%s" for %s, ' 'it is already handled by %s' ) % (scheme, name(b), name(backends_by_scheme[scheme])) backends_by_scheme[scheme] = b From 9f08bce6cd51ce8dbfa25cccab6b618cbb48ed1e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 11 Aug 2015 10:06:58 +0200 Subject: [PATCH 60/94] core: Update test --- tests/core/test_actor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index 410933d2..8f062fa2 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -37,7 +37,8 @@ class CoreActorTest(unittest.TestCase): self.assertRaisesRegexp( AssertionError, - 'Cannot add URI scheme dummy1 for B2, it is already handled by B1', + 'Cannot add URI scheme "dummy1" for B2, ' + 'it is already handled by B1', Core, mixer=None, backends=[self.backend1, self.backend2]) def test_version(self): From 25e612876cd2a5ffdf8be9061d87e74b6b37352f Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 12 Aug 2015 22:21:01 +0200 Subject: [PATCH 61/94] docs: Add captions to ToCs --- docs/index.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/index.rst b/docs/index.rst index 9085024a..647d2319 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -78,6 +78,7 @@ Usage ===== .. toctree:: + :caption: Usage :maxdepth: 2 installation/index @@ -93,6 +94,7 @@ Extensions ========== .. toctree:: + :caption: Extensions :maxdepth: 2 ext/local @@ -112,6 +114,7 @@ Clients ======= .. toctree:: + :caption: Clients :maxdepth: 2 clients/http @@ -124,6 +127,7 @@ About ===== .. toctree:: + :caption: About :maxdepth: 1 authors @@ -136,6 +140,7 @@ Development =========== .. toctree:: + :caption: Development :maxdepth: 2 contributing @@ -149,6 +154,7 @@ Reference ========= .. toctree:: + :caption: Reference :maxdepth: 2 glossary From d942ae0555398630cc7f064ad364c5a5df5f7735 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 12 Aug 2015 22:23:32 +0200 Subject: [PATCH 62/94] docs: Remove headers made redundant by ToC captions --- docs/index.rst | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 647d2319..70d14a73 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -74,9 +74,6 @@ If you want to stay up to date on Mopidy developments, you can follow `@mopidy announcements related to Mopidy and Mopidy extensions. -Usage -===== - .. toctree:: :caption: Usage :maxdepth: 2 @@ -90,9 +87,6 @@ Usage .. _ext: -Extensions -========== - .. toctree:: :caption: Extensions :maxdepth: 2 @@ -110,9 +104,6 @@ Extensions ext/web -Clients -======= - .. toctree:: :caption: Clients :maxdepth: 2 @@ -123,9 +114,6 @@ Clients clients/upnp -About -===== - .. toctree:: :caption: About :maxdepth: 1 @@ -136,9 +124,6 @@ About versioning -Development -=========== - .. toctree:: :caption: Development :maxdepth: 2 @@ -150,9 +135,6 @@ Development extensiondev -Reference -========= - .. toctree:: :caption: Reference :maxdepth: 2 From 24d3fed411ebf3ce5a9eebc5971c5c1c0878cd5b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 12 Aug 2015 22:21:01 +0200 Subject: [PATCH 63/94] docs: Add captions to ToCs (cherry picked from commit 25e612876cd2a5ffdf8be9061d87e74b6b37352f) --- docs/index.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/index.rst b/docs/index.rst index 9085024a..647d2319 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -78,6 +78,7 @@ Usage ===== .. toctree:: + :caption: Usage :maxdepth: 2 installation/index @@ -93,6 +94,7 @@ Extensions ========== .. toctree:: + :caption: Extensions :maxdepth: 2 ext/local @@ -112,6 +114,7 @@ Clients ======= .. toctree:: + :caption: Clients :maxdepth: 2 clients/http @@ -124,6 +127,7 @@ About ===== .. toctree:: + :caption: About :maxdepth: 1 authors @@ -136,6 +140,7 @@ Development =========== .. toctree:: + :caption: Development :maxdepth: 2 contributing @@ -149,6 +154,7 @@ Reference ========= .. toctree:: + :caption: Reference :maxdepth: 2 glossary From f5f957e4c9b187dc78d12b00dcc3e3c75216cf9c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 12 Aug 2015 22:23:32 +0200 Subject: [PATCH 64/94] docs: Remove headers made redundant by ToC captions (cherry picked from commit d942ae0555398630cc7f064ad364c5a5df5f7735) --- docs/index.rst | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 647d2319..70d14a73 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -74,9 +74,6 @@ If you want to stay up to date on Mopidy developments, you can follow `@mopidy announcements related to Mopidy and Mopidy extensions. -Usage -===== - .. toctree:: :caption: Usage :maxdepth: 2 @@ -90,9 +87,6 @@ Usage .. _ext: -Extensions -========== - .. toctree:: :caption: Extensions :maxdepth: 2 @@ -110,9 +104,6 @@ Extensions ext/web -Clients -======= - .. toctree:: :caption: Clients :maxdepth: 2 @@ -123,9 +114,6 @@ Clients clients/upnp -About -===== - .. toctree:: :caption: About :maxdepth: 1 @@ -136,9 +124,6 @@ About versioning -Development -=========== - .. toctree:: :caption: Development :maxdepth: 2 @@ -150,9 +135,6 @@ Development extensiondev -Reference -========= - .. toctree:: :caption: Reference :maxdepth: 2 From b1c4324def1a53bd90d999b3a5de39243b21b762 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 13 Aug 2015 09:34:58 +0200 Subject: [PATCH 65/94] docs: Update sponsors page --- docs/sponsors.rst | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/sponsors.rst b/docs/sponsors.rst index dc94aa6f..2d8b7f4e 100644 --- a/docs/sponsors.rst +++ b/docs/sponsors.rst @@ -20,13 +20,21 @@ for free. We use their services for the following sites: - Mailgun for sending emails from the Discourse forum. -- CDN hosting at http://dl.mopidy.com, which is used to distribute Pi Musicbox + +Fastly +====== + +`Fastly `_ lets Mopidy use their CDN for free. We +accelerate requests to all Mopidy services, including: + +- https://apt.mopidy.com/dists/, which is used to distribute Debian packages. + +- https://dl.mopidy.com/pimusicbox/, which is used to distribute Pi Musicbox images. GlobalSign ========== -`GlobalSign `_ provides Mopidy with a free -wildcard SSL certificate for mopidy.com, which we use to secure access to all -our web sites. +`GlobalSign `_ provides Mopidy with a free SSL +certificate for mopidy.com, which we use to secure access to all our web sites. From 96d5039054e4ceb2cd137d177007e339055ec302 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 13 Aug 2015 09:34:58 +0200 Subject: [PATCH 66/94] docs: Update sponsors page (cherry picked from commit b1c4324def1a53bd90d999b3a5de39243b21b762) --- docs/sponsors.rst | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/sponsors.rst b/docs/sponsors.rst index dc94aa6f..2d8b7f4e 100644 --- a/docs/sponsors.rst +++ b/docs/sponsors.rst @@ -20,13 +20,21 @@ for free. We use their services for the following sites: - Mailgun for sending emails from the Discourse forum. -- CDN hosting at http://dl.mopidy.com, which is used to distribute Pi Musicbox + +Fastly +====== + +`Fastly `_ lets Mopidy use their CDN for free. We +accelerate requests to all Mopidy services, including: + +- https://apt.mopidy.com/dists/, which is used to distribute Debian packages. + +- https://dl.mopidy.com/pimusicbox/, which is used to distribute Pi Musicbox images. GlobalSign ========== -`GlobalSign `_ provides Mopidy with a free -wildcard SSL certificate for mopidy.com, which we use to secure access to all -our web sites. +`GlobalSign `_ provides Mopidy with a free SSL +certificate for mopidy.com, which we use to secure access to all our web sites. From 9a83a2d707bd430a0529b1ae889ec1e8b9604f04 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 15 Aug 2015 22:34:59 +0200 Subject: [PATCH 67/94] stream: Ignore file protocol if Mopidy-File is enabled If Mopidy-File is enabled it handles playback of file:// URIs. Mopidy-Stream used to do this, but in Mopidy 1.1 we removed "file" from the default value of the stream/protocols config. However, many users upgrading to Mopidy 1.1 have set stream/protocols to include "file" in their existing config, and thus Mopidy fails to start because both backends tries to claim the "file" protocol. Fixes #1248 --- mopidy/stream/actor.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/mopidy/stream/actor.py b/mopidy/stream/actor.py index ae5be8e0..cc9632d5 100644 --- a/mopidy/stream/actor.py +++ b/mopidy/stream/actor.py @@ -36,6 +36,13 @@ class StreamBackend(pykka.ThreadingActor, backend.Backend): self.uri_schemes = audio_lib.supported_uri_schemes( config['stream']['protocols']) + if 'file' in self.uri_schemes and config['file']['enabled']: + logger.warning( + 'The stream/protocols config value includes the "file" ' + 'protocol. "file" playback is now handled by Mopidy-File. ' + 'Please remove it from the stream/protocols config.') + self.uri_schemes -= {'file'} + class StreamLibraryProvider(backend.LibraryProvider): From 3dfa39adb04b5e675e1ebe0c312bbfb3240f6a05 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 15 Aug 2015 22:50:48 +0200 Subject: [PATCH 68/94] docs: Add #1248/#1254 to changelog --- docs/changelog.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 75fdc538..24bc7682 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,19 @@ Changelog This changelog is used to track all major changes to Mopidy. + +v1.1.1 (UNRELEASED) +=================== + +Bug fix release. + +- Stream: If "file" is present in the :confval:`stream/protocols` config value + and the :ref:`ext-file` extension is enabled, we exited with an error because + two extensions claimed the same URI scheme. We now log a warning recommending + to remove "file" from the :confval:`stream/protocols` config, and then + proceed startup. (Fixes: :issue:`1248`, PR: :issue:`1254`) + + v1.1.0 (2015-08-09) =================== From 9f24c331a4ea2237579dce9b6f06390ad2b10411 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 15 Aug 2015 23:08:32 +0200 Subject: [PATCH 69/94] file: Adjust file/media_dirs failure logging Fixes #1249 --- mopidy/file/library.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/mopidy/file/library.py b/mopidy/file/library.py index 691020b4..d477d109 100644 --- a/mopidy/file/library.py +++ b/mopidy/file/library.py @@ -108,12 +108,15 @@ class FileLibraryProvider(backend.LibraryProvider): media_dir_split[0].encode(FS_ENCODING)) if not local_path: - logger.warning('Failed expanding path (%s) from ' - 'file/media_dirs config value.', - media_dir_split[0]) + logger.debug( + 'Failed expanding path (%s) from file/media_dirs config ' + 'value.', + media_dir_split[0]) continue elif not os.path.isdir(local_path): - logger.warning('%s is not a directory', local_path) + logger.warning( + '%s is not a directory. Please create the directory or ' + 'update the file/media_dirs config value.', local_path) continue media_dir['path'] = local_path From 306b2f331fba29c073ce07691d1408e139c8914d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 15 Aug 2015 23:11:47 +0200 Subject: [PATCH 70/94] docs: Add #1249/#1255 to changelog --- docs/changelog.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 75fdc538..7a604526 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,18 @@ Changelog This changelog is used to track all major changes to Mopidy. + +v1.1.1 (UNRELEASED) +=================== + +Bug fix release. + +- File: Adjust log levels when failing to expand ``$XDG_MUSIC_DIR`` into a real + path. This usually happens when running Mopidy as a system service, and thus + with a limited set of environment variables. (Fixes: :issue:`1249`, PR: + :issue:`1255`) + + v1.1.0 (2015-08-09) =================== From 087ee4288246b6586f5ab0fcd916e81bfeda9137 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 16 Aug 2015 12:06:14 +0200 Subject: [PATCH 71/94] audio: Fix scan timeout handling --- docs/changelog.rst | 3 +++ mopidy/audio/scan.py | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c7f1aeee..290806d3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,6 +21,9 @@ Bug fix release. with a limited set of environment variables. (Fixes: :issue:`1249`, PR: :issue:`1255`) +- Audio: Fix timeout handling in scanner. This regression caused timeouts to + expire before it should, causing scans to fail. + v1.1.0 (2015-08-09) =================== diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index cf370052..13c76d52 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -139,7 +139,7 @@ def _process(pipeline, timeout_ms): types = (gst.MESSAGE_ELEMENT | gst.MESSAGE_APPLICATION | gst.MESSAGE_ERROR | gst.MESSAGE_EOS | gst.MESSAGE_ASYNC_DONE | gst.MESSAGE_TAG) - start = clock.get_time() + previous = clock.get_time() while timeout > 0: message = bus.timed_pop_filtered(timeout, types) @@ -171,7 +171,9 @@ def _process(pipeline, timeout_ms): # Note that this will only keep the last tag. tags.update(utils.convert_taglist(taglist)) - timeout -= clock.get_time() - start + now = clock.get_time() + timeout -= now - previous + previous = now raise exceptions.ScannerError('Timeout after %dms' % timeout_ms) From e77a4afaf46f2eff8a700d6eb73082bbb93be419 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 18 Aug 2015 23:44:38 +0200 Subject: [PATCH 72/94] audio: Make scanner report MIME for missing plugins --- docs/changelog.rst | 3 +++ mopidy/audio/scan.py | 13 ++++++------- tests/audio/test_scan.py | 11 +++++++++++ tests/data/scanner/plain.txt | 1 + tests/data/scanner/playlist.m3u | 1 + 5 files changed, 22 insertions(+), 7 deletions(-) create mode 100644 tests/data/scanner/plain.txt create mode 100644 tests/data/scanner/playlist.m3u diff --git a/docs/changelog.rst b/docs/changelog.rst index 290806d3..c0328de5 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,9 @@ Bug fix release. - Audio: Fix timeout handling in scanner. This regression caused timeouts to expire before it should, causing scans to fail. +- Audio: Update scanner to emit MIME type instead of an error when missing a + plugin. + v1.1.0 (2015-08-09) =================== diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index 13c76d52..d1081788 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -12,8 +12,6 @@ from mopidy import exceptions from mopidy.audio import utils from mopidy.internal import encoding -_missing_plugin_desc = gst.pbutils.missing_plugin_message_get_description - _Result = collections.namedtuple( 'Result', ('uri', 'tags', 'duration', 'seekable', 'mime', 'playable')) @@ -134,7 +132,7 @@ def _process(pipeline, timeout_ms): clock = pipeline.get_clock() bus = pipeline.get_bus() timeout = timeout_ms * gst.MSECOND - tags, mime, have_audio, missing_description = {}, None, False, None + tags, mime, have_audio, missing_message = {}, None, False, None types = (gst.MESSAGE_ELEMENT | gst.MESSAGE_APPLICATION | gst.MESSAGE_ERROR | gst.MESSAGE_EOS | gst.MESSAGE_ASYNC_DONE | gst.MESSAGE_TAG) @@ -147,8 +145,7 @@ def _process(pipeline, timeout_ms): break elif message.type == gst.MESSAGE_ELEMENT: if gst.pbutils.is_missing_plugin_message(message): - missing_description = encoding.locale_decode( - _missing_plugin_desc(message)) + missing_message = message elif message.type == gst.MESSAGE_APPLICATION: if message.structure.get_name() == 'have-type': mime = message.structure['caps'].get_name() @@ -158,8 +155,10 @@ def _process(pipeline, timeout_ms): have_audio = True elif message.type == gst.MESSAGE_ERROR: error = encoding.locale_decode(message.parse_error()[0]) - if missing_description: - error = '%s (%s)' % (missing_description, error) + if missing_message and not mime: + caps = missing_message.structure['detail'] + mime = caps.get_structure(0).get_name() + return tags, mime, have_audio raise exceptions.ScannerError(error) elif message.type == gst.MESSAGE_EOS: return tags, mime, have_audio diff --git a/tests/audio/test_scan.py b/tests/audio/test_scan.py index c558835e..dff753d9 100644 --- a/tests/audio/test_scan.py +++ b/tests/audio/test_scan.py @@ -109,6 +109,17 @@ class ScannerTest(unittest.TestCase): wav = path_to_data_dir('scanner/empty.wav') self.assertEqual(self.result[wav].duration, 0) + def test_uri_list(self): + path = path_to_data_dir('scanner/playlist.m3u') + self.scan([path]) + self.assertEqual(self.result[path].mime, 'text/uri-list') + + def test_text_plain(self): + # GStreamer decode bin hardcodes bad handling of text plain :/ + path = path_to_data_dir('scanner/plain.txt') + self.scan([path]) + self.assertIn(path, self.errors) + @unittest.SkipTest def test_song_without_time_is_handeled(self): pass diff --git a/tests/data/scanner/plain.txt b/tests/data/scanner/plain.txt new file mode 100644 index 00000000..3180d4a9 --- /dev/null +++ b/tests/data/scanner/plain.txt @@ -0,0 +1 @@ +Some plain text file with nothing special in it. diff --git a/tests/data/scanner/playlist.m3u b/tests/data/scanner/playlist.m3u new file mode 100644 index 00000000..df0f0142 --- /dev/null +++ b/tests/data/scanner/playlist.m3u @@ -0,0 +1 @@ +http://example.com/ From 3e75d5cf06ccccc18077b9cce2072d4dd6c3267c Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 19 Aug 2015 00:40:46 +0200 Subject: [PATCH 73/94] audio: Update missing plugins check in scanner tests --- tests/audio/test_scan.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/audio/test_scan.py b/tests/audio/test_scan.py index dff753d9..8c2b9af3 100644 --- a/tests/audio/test_scan.py +++ b/tests/audio/test_scan.py @@ -40,8 +40,11 @@ class ScannerTest(unittest.TestCase): self.assertEqual(self.result[name].tags[key], value) def check_if_missing_plugin(self): - if any(['missing a plug-in' in str(e) for e in self.errors.values()]): - raise unittest.SkipTest('Missing MP3 support?') + for path, result in self.result.items(): + if not path.endswith('.mp3'): + continue + if not result.playable and result.mime == 'audio/mpeg': + raise unittest.SkipTest('Missing MP3 support?') def test_tags_is_set(self): self.scan(self.find('scanner/simple')) From 74e4d3f9ab1c2dd869966ee975de75aeab984985 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 20 Aug 2015 10:28:13 +0200 Subject: [PATCH 74/94] docs: pip-mopidy can now use apt-extensions --- docs/debian.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/debian.rst b/docs/debian.rst index f939d9af..74bab30f 100644 --- a/docs/debian.rst +++ b/docs/debian.rst @@ -114,11 +114,17 @@ from a regular Mopidy setup you'll want to know about. sudo service mopidy status - Mopidy installed from a Debian package can use both Mopidy extensions - installed both from Debian packages and extensions installed with pip. + installed both from Debian packages and extensions installed with pip. This + has always been the case. - The other way around does not work: Mopidy installed with pip can use - extensions installed with pip, but not extensions installed from a Debian - package. This is because the Debian packages install extensions into + Mopidy installed with pip can use extensions installed with pip, but + not extensions installed from a Debian package released before August 2015. + This is because the Debian packages used to install extensions into :file:`/usr/share/mopidy` which is normally not on your ``PYTHONPATH``. - Thus, your pip-installed Mopidy will not find the Debian package-installed + Thus, your pip-installed Mopidy would not find the Debian package-installed extensions. + + In August 2015, all Mopidy extension Debian packages was modified to install + into :file:`/usr/lib/python2.7/dist-packages`, like any other Python Debian + package. Thus, Mopidy installed with pip can now use extensions installed + from Debian. From 7411c84ba7bb306f3fe06e4cbb9ed2223c0363d5 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 20 Aug 2015 10:50:35 +0200 Subject: [PATCH 75/94] docs: Avoid double use of 'both' in same sentence --- docs/debian.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/debian.rst b/docs/debian.rst index 74bab30f..f761c4b0 100644 --- a/docs/debian.rst +++ b/docs/debian.rst @@ -113,9 +113,8 @@ from a regular Mopidy setup you'll want to know about. sudo service mopidy status -- Mopidy installed from a Debian package can use both Mopidy extensions - installed both from Debian packages and extensions installed with pip. This - has always been the case. +- Mopidy installed from a Debian package can use Mopidy extensions installed + both from Debian packages and with pip. This has always been the case. Mopidy installed with pip can use extensions installed with pip, but not extensions installed from a Debian package released before August 2015. From 77e8436f3e2b09d85eaeb2d89d6ef1dd96eaf1bb Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Aug 2015 19:57:39 +0200 Subject: [PATCH 76/94] docs: Elaborate on core/{cache,data}_dir usage Related to #1253 --- docs/config.rst | 12 ++++++++++++ mopidy/ext.py | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/docs/config.rst b/docs/config.rst index 26304176..7f0bda31 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -65,6 +65,13 @@ Core configuration Path to base directory for storing cached data. + Mopidy and extensions will use this path to cache data that can safely be + thrown away. + + If your system is running from an SD card, it can help avoid wear and + corruption of your SD card by pointing this config to another location. If + you have enough RAM, a tmpfs might be a good choice. + When running Mopidy as a regular user, this should usually be ``$XDG_CACHE_DIR/mopidy``, i.e. :file:`~/.cache/mopidy`. @@ -85,6 +92,11 @@ Core configuration Path to base directory for persistent data files. + Mopidy and extensions will use this path to store data that cannot be + be thrown away and reproduced without some effort. Examples include + Mopidy-Local's index of your media library and Mopidy-M3U's stored + playlists. + When running Mopidy as a regular user, this should usually be ``$XDG_DATA_DIR/mopidy``, i.e. :file:`~/.local/share/mopidy`. diff --git a/mopidy/ext.py b/mopidy/ext.py index 908b6d5d..199d7ab6 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -63,6 +63,8 @@ class Extension(object): def get_cache_dir(self, config): """Get or create cache directory for the extension. + Use this directory to cache data that can safely be thrown away. + :param config: the Mopidy config object :return: string """ @@ -87,6 +89,8 @@ class Extension(object): def get_data_dir(self, config): """Get or create data directory for the extension. + Use this directory to store data that should be persistent. + :param config: the Mopidy config object :returns: string """ From eee851f36beeee5b5072d1716bd560f7529099c8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Aug 2015 20:34:10 +0200 Subject: [PATCH 77/94] mpd: Fix missing punctuation in docstring --- mopidy/mpd/dispatcher.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py index a8e2c05c..099a2f18 100644 --- a/mopidy/mpd/dispatcher.py +++ b/mopidy/mpd/dispatcher.py @@ -271,9 +271,9 @@ class MpdContext(object): If ``lookup`` is true and the ``path`` is to a track, the returned ``data`` is a future which will contain the results from looking up - the URI with :meth:`mopidy.core.LibraryController.lookup` If ``lookup`` - is false and the ``path`` is to a track, the returned ``data`` will be - a :class:`mopidy.models.Ref` for the track. + the URI with :meth:`mopidy.core.LibraryController.lookup`. If + ``lookup`` is false and the ``path`` is to a track, the returned + ``data`` will be a :class:`mopidy.models.Ref` for the track. For all entries that are not tracks, the returned ``data`` will be :class:`None`. From 52b81bd858af62f3a01e781e068067d2af4ca900 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Aug 2015 20:40:18 +0200 Subject: [PATCH 78/94] file: Don't scan files on browsing Fixes #1260 --- docs/changelog.rst | 5 +++++ mopidy/file/library.py | 14 +------------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c0328de5..6ab7e732 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,6 +21,11 @@ Bug fix release. with a limited set of environment variables. (Fixes: :issue:`1249`, PR: :issue:`1255`) +- File: When browsing files, we no longer scan the files to check if they're + playable. This makes browsing of the file hierarchy instant for HTTP clients, + which do no scanning of the files' metadata, and a bit faster for MPD + clients, which no longer scan the files twice. + - Audio: Fix timeout handling in scanner. This regression caused timeouts to expire before it should, causing scans to fail. diff --git a/mopidy/file/library.py b/mopidy/file/library.py index d477d109..b8531a6e 100644 --- a/mopidy/file/library.py +++ b/mopidy/file/library.py @@ -71,7 +71,7 @@ class FileLibraryProvider(backend.LibraryProvider): name = dir_entry.decode(FS_ENCODING, 'replace') if os.path.isdir(child_path): result.append(models.Ref.directory(name=name, uri=uri)) - elif os.path.isfile(child_path) and self._is_audio_file(uri): + elif os.path.isfile(child_path): result.append(models.Ref.track(name=name, uri=uri)) result.sort(key=operator.attrgetter('name')) @@ -134,18 +134,6 @@ class FileLibraryProvider(backend.LibraryProvider): name=media_dir['name'], uri=path.path_to_uri(media_dir['path'])) - def _is_audio_file(self, uri): - try: - result = self._scanner.scan(uri) - if result.playable: - logger.debug('Playable file: %s', result.uri) - else: - logger.debug('Unplayable file: %s (not audio)', result.uri) - return result.playable - except exceptions.ScannerError as e: - logger.debug('Unplayable file: %s (%s)', uri, e) - return False - def _is_in_basedir(self, local_path): return any( path.is_path_inside_base_dir(local_path, media_dir['path']) From baa2cc7ac845209aab5d2fb4b41fed179cf5c9ae Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Aug 2015 20:44:25 +0200 Subject: [PATCH 79/94] docs: Add issue and PR links --- docs/changelog.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6ab7e732..55357087 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,7 +24,8 @@ Bug fix release. - File: When browsing files, we no longer scan the files to check if they're playable. This makes browsing of the file hierarchy instant for HTTP clients, which do no scanning of the files' metadata, and a bit faster for MPD - clients, which no longer scan the files twice. + clients, which no longer scan the files twice. (Fixes: :issue:`1260`, PR: + :issue:`1261`) - Audio: Fix timeout handling in scanner. This regression caused timeouts to expire before it should, causing scans to fail. From 9bc78ac10e443bd74998a659eb9a66032d727b23 Mon Sep 17 00:00:00 2001 From: Ben Evans Date: Sat, 22 Aug 2015 01:06:30 +0100 Subject: [PATCH 80/94] docs: JS block fix --- docs/api/js.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/js.rst b/docs/api/js.rst index 6a8e0fcd..856f2db4 100644 --- a/docs/api/js.rst +++ b/docs/api/js.rst @@ -256,7 +256,7 @@ chain. The function will be called with the error object as the only argument: .. code-block:: js mopidy.playback.getCurrentTrack() - .catch(console.error.bind(console)); + .catch(console.error.bind(console)) .done(printCurrentTrack); You can also register the error handler at the end of the promise chain by From 78ffaeb8d29f92d9c107780bfe43017a85fff7a9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 22 Aug 2015 23:51:07 +0200 Subject: [PATCH 81/94] playlists: Fix crash on urilist comment with non-ASCII chars Fixes #1265 --- docs/changelog.rst | 4 ++++ mopidy/internal/playlists.py | 2 +- tests/internal/test_playlists.py | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 55357087..a310e7e8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,10 @@ Bug fix release. to remove "file" from the :confval:`stream/protocols` config, and then proceed startup. (Fixes: :issue:`1248`, PR: :issue:`1254`) +- Stream: Fix bug in new playlist parser. A non-ASCII char in an urilist + comment would cause a crash while parsing due to comparision of a non-ASCII + bytestring with a Unicode string. (Fixes: :issue:`1265`) + - File: Adjust log levels when failing to expand ``$XDG_MUSIC_DIR`` into a real path. This usually happens when running Mopidy as a system service, and thus with a limited set of environment variables. (Fixes: :issue:`1249`, PR: diff --git a/mopidy/internal/playlists.py b/mopidy/internal/playlists.py index 219d3ec6..f8e654af 100644 --- a/mopidy/internal/playlists.py +++ b/mopidy/internal/playlists.py @@ -122,7 +122,7 @@ def parse_asx(data): def parse_urilist(data): result = [] for line in data.splitlines(): - if not line.strip() or line.startswith('#'): + if not line.strip() or line.startswith(b'#'): continue try: validation.check_uri(line) diff --git a/tests/internal/test_playlists.py b/tests/internal/test_playlists.py index 21537813..9a1c49d5 100644 --- a/tests/internal/test_playlists.py +++ b/tests/internal/test_playlists.py @@ -23,7 +23,7 @@ file:///tmp/baz URILIST = b""" file:///tmp/foo -# a comment +# a comment \xc5\xa7\xc5\x95 file:///tmp/bar file:///tmp/baz From c48b6515f99751ef80203cc7dc3fdfec4f16993a Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Aug 2015 00:35:52 +0200 Subject: [PATCH 82/94] core: library.refresh() should check if backend has library ...and not playlists. Fixes #1257 --- docs/changelog.rst | 4 ++++ mopidy/core/library.py | 2 +- tests/core/test_library.py | 6 ++++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index a310e7e8..c8007e7b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,6 +10,10 @@ v1.1.1 (UNRELEASED) Bug fix release. +- Core: Make :meth:`mopidy.core.LibraryController.refresh` work for all + backends with a library provider. Previously, it wrongly worked for all + backends with a playlists provider. (Fixes: :issue:`1257`) + - Stream: If "file" is present in the :confval:`stream/protocols` config value and the :ref:`ext-file` extension is enabled, we exited with an error because two extensions claimed the same URI scheme. We now log a warning recommending diff --git a/mopidy/core/library.py b/mopidy/core/library.py index c300fbb9..ce420812 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -255,7 +255,7 @@ class LibraryController(object): backends = {} uri_scheme = urlparse.urlparse(uri).scheme if uri else None - for backend_scheme, backend in self.backends.with_playlists.items(): + for backend_scheme, backend in self.backends.with_library.items(): backends.setdefault(backend, set()).add(backend_scheme) for backend, backend_schemes in backends.items(): diff --git a/tests/core/test_library.py b/tests/core/test_library.py index 941f1831..92b22bfb 100644 --- a/tests/core/test_library.py +++ b/tests/core/test_library.py @@ -20,6 +20,7 @@ class BaseCoreLibraryTest(unittest.TestCase): self.library1.get_images.return_value.get.return_value = {} self.library1.root_directory.get.return_value = dummy1_root self.backend1.library = self.library1 + self.backend1.has_playlists.return_value.get.return_value = False dummy2_root = Ref.directory(uri='dummy2:directory', name='dummy2') self.backend2 = mock.Mock() @@ -29,13 +30,14 @@ class BaseCoreLibraryTest(unittest.TestCase): self.library2.get_images.return_value.get.return_value = {} self.library2.root_directory.get.return_value = dummy2_root self.backend2.library = self.library2 + self.backend2.has_playlists.return_value.get.return_value = False # A backend without the optional library provider self.backend3 = mock.Mock() self.backend3.uri_schemes.get.return_value = ['dummy3'] self.backend3.actor_ref.actor_class.__name__ = 'DummyBackend3' - self.backend3.has_library().get.return_value = False - self.backend3.has_library_browse().get.return_value = False + self.backend3.has_library.return_value.get.return_value = False + self.backend3.has_library_browse.return_value.get.return_value = False self.core = core.Core(mixer=None, backends=[ self.backend1, self.backend2, self.backend3]) From b63642d873111a1dc41b8a61f407b7f7330a4f5d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 22 Aug 2015 21:34:35 +0200 Subject: [PATCH 83/94] Use core/*_dir when creating dirs we need Partly fixes #1259 --- docs/changelog.rst | 9 +++++++++ mopidy/__main__.py | 20 +++++++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c8007e7b..5caa176a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -14,6 +14,15 @@ Bug fix release. backends with a library provider. Previously, it wrongly worked for all backends with a playlists provider. (Fixes: :issue:`1257`) +- Core: Respect :confval:`core/cache_dir` and :confval:`core/data_dir` config + values added in 1.1.0 when creating the dirs Mopidy need to store data. This + should not change the behavior for desktop users running Mopidy. When running + Mopidy as a system service installed from a package which sets the core dir + configs properly (e.g. Debian and Arch packages), this fix avoids the + creation of a couple of directories that should not be used, typically + :file:`/var/lib/mopidy/.local` and :file:`/var/lib/mopidy/.cache`. (Fixes: + :issue:`1259`) + - Stream: If "file" is present in the :confval:`stream/protocols` config value and the :ref:`ext-file` extension is enabled, we exited with an error because two extensions claimed the same URI scheme. We now log a warning recommending diff --git a/mopidy/__main__.py b/mopidy/__main__.py index ee359268..208d2ff1 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -75,15 +75,16 @@ def main(): args = root_cmd.parse(mopidy_args) - create_file_structures_and_config(args, extensions_data) - check_old_locations() - config, config_errors = config_lib.load( args.config_files, [d.config_schema for d in extensions_data], [d.config_defaults for d in extensions_data], args.config_overrides) + create_core_dirs(config) + create_initial_config_file(args, extensions_data) + check_old_locations() + verbosity_level = args.base_verbosity_level if args.verbosity_level: verbosity_level += args.verbosity_level @@ -166,12 +167,17 @@ def main(): raise -def create_file_structures_and_config(args, extensions_data): - path.get_or_create_dir(b'$XDG_DATA_DIR/mopidy') - path.get_or_create_dir(b'$XDG_CONFIG_DIR/mopidy') +def create_core_dirs(config): + path.get_or_create_dir(config['core']['cache_dir']) + path.get_or_create_dir(config['core']['config_dir']) + path.get_or_create_dir(config['core']['data_dir']) + + +def create_initial_config_file(args, extensions_data): + """Initialize whatever the last config file is with defaults""" - # Initialize whatever the last config file is with defaults config_file = args.config_files[-1] + if os.path.exists(path.expand_path(config_file)): return From e0a028291a453ca49d879c7dfcaa67488394938d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 22 Aug 2015 21:49:18 +0200 Subject: [PATCH 84/94] local: Replace local/data_dir with core/data_dir Partly fixes #1259 --- docs/changelog.rst | 6 ++++++ docs/ext/local.rst | 4 ++-- mopidy/local/__init__.py | 2 +- mopidy/local/ext.conf | 1 - mopidy/local/json.py | 4 ++-- mopidy/local/storage.py | 9 --------- tests/data/{ => local}/library.json.gz | Bin tests/local/test_json.py | 5 +++-- tests/local/test_library.py | 18 ++++++++++++------ tests/local/test_playback.py | 3 +-- tests/local/test_tracklist.py | 2 +- 11 files changed, 28 insertions(+), 26 deletions(-) rename tests/data/{ => local}/library.json.gz (100%) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5caa176a..9e1e5fec 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -23,6 +23,12 @@ Bug fix release. :file:`/var/lib/mopidy/.local` and :file:`/var/lib/mopidy/.cache`. (Fixes: :issue:`1259`) +- Local: Deprecate :confval:`local/data_dir` and respect + :confval:`core/data_dir` instead. This does not change the defaults for + desktop users, only system services installed from packages that properly set + :confval:`core/data_dir`, like the Debian and Arch packages. (Fixes: + :issue:`1259`) + - Stream: If "file" is present in the :confval:`stream/protocols` config value and the :ref:`ext-file` extension is enabled, we exited with an error because two extensions claimed the same URI scheme. We now log a warning recommending diff --git a/docs/ext/local.rst b/docs/ext/local.rst index 18f66adc..d6e3b56a 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -47,8 +47,8 @@ active at a time. To create a new library provider you must create class that implements the :class:`mopidy.local.Library` interface and install it in the extension registry under ``local:library``. Any data that the library needs to store on -disc should be stored in :confval:`local/data_dir` using the library name as -part of the filename or directory to avoid any conflicts. +disc should be stored in the extension's data dir, as returned by +:meth:`~mopidy.ext.Extension.get_data_dir`. Configuration diff --git a/mopidy/local/__init__.py b/mopidy/local/__init__.py index ff61c17c..3ee2703e 100644 --- a/mopidy/local/__init__.py +++ b/mopidy/local/__init__.py @@ -23,7 +23,7 @@ class Extension(ext.Extension): schema = super(Extension, self).get_config_schema() schema['library'] = config.String() schema['media_dir'] = config.Path() - schema['data_dir'] = config.Path() + schema['data_dir'] = config.Deprecated() schema['playlists_dir'] = config.Deprecated() schema['tag_cache_file'] = config.Deprecated() schema['scan_timeout'] = config.Integer( diff --git a/mopidy/local/ext.conf b/mopidy/local/ext.conf index ebd7962f..6dd29147 100644 --- a/mopidy/local/ext.conf +++ b/mopidy/local/ext.conf @@ -2,7 +2,6 @@ enabled = true library = json media_dir = $XDG_MUSIC_DIR -data_dir = $XDG_DATA_DIR/mopidy/local scan_timeout = 1000 scan_flush_threshold = 1000 scan_follow_symlinks = false diff --git a/mopidy/local/json.py b/mopidy/local/json.py index 0be5e99e..501990ee 100644 --- a/mopidy/local/json.py +++ b/mopidy/local/json.py @@ -12,7 +12,7 @@ import tempfile import mopidy from mopidy import compat, local, models from mopidy.internal import encoding, timer -from mopidy.local import search, storage, translator +from mopidy.local import Extension, search, storage, translator logger = logging.getLogger(__name__) @@ -116,7 +116,7 @@ class JsonLibrary(local.Library): self._browse_cache = None self._media_dir = config['local']['media_dir'] self._json_file = os.path.join( - config['local']['data_dir'], b'library.json.gz') + Extension().get_data_dir(config), b'library.json.gz') storage.check_dirs_and_files(config) diff --git a/mopidy/local/storage.py b/mopidy/local/storage.py index 1808c4a2..aaa74fba 100644 --- a/mopidy/local/storage.py +++ b/mopidy/local/storage.py @@ -3,8 +3,6 @@ from __future__ import absolute_import, unicode_literals import logging import os -from mopidy.internal import encoding, path - logger = logging.getLogger(__name__) @@ -13,10 +11,3 @@ def check_dirs_and_files(config): logger.warning( 'Local media dir %s does not exist.' % config['local']['media_dir']) - - try: - path.get_or_create_dir(config['local']['data_dir']) - except EnvironmentError as error: - logger.warning( - 'Could not create local data dir: %s', - encoding.locale_decode(error)) diff --git a/tests/data/library.json.gz b/tests/data/local/library.json.gz similarity index 100% rename from tests/data/library.json.gz rename to tests/data/local/library.json.gz diff --git a/tests/local/test_json.py b/tests/local/test_json.py index 520287ad..169ba743 100644 --- a/tests/local/test_json.py +++ b/tests/local/test_json.py @@ -45,10 +45,11 @@ class BrowseCacheTest(unittest.TestCase): class JsonLibraryTest(unittest.TestCase): config = { + 'core': { + 'data_dir': path_to_data_dir(''), + }, 'local': { 'media_dir': path_to_data_dir(''), - 'data_dir': path_to_data_dir(''), - 'playlists_dir': b'', 'library': 'json', }, } diff --git a/tests/local/test_library.py b/tests/local/test_library.py index 7763057f..4142d6c3 100644 --- a/tests/local/test_library.py +++ b/tests/local/test_library.py @@ -65,10 +65,11 @@ class LocalLibraryProviderTest(unittest.TestCase): ] config = { + 'core': { + 'data_dir': path_to_data_dir(''), + }, 'local': { 'media_dir': path_to_data_dir(''), - 'data_dir': path_to_data_dir(''), - 'playlists_dir': b'', 'library': 'json', }, } @@ -105,11 +106,15 @@ class LocalLibraryProviderTest(unittest.TestCase): tmpdir = tempfile.mkdtemp() try: - tmplib = os.path.join(tmpdir, 'library.json.gz') - shutil.copy(path_to_data_dir('library.json.gz'), tmplib) + tmpdir_local = os.path.join(tmpdir, 'local') + shutil.copytree(path_to_data_dir('local'), tmpdir_local) - config = {'local': self.config['local'].copy()} - config['local']['data_dir'] = tmpdir + config = { + 'core': { + 'data_dir': tmpdir, + }, + 'local': self.config['local'], + } backend = actor.LocalBackend(config=config, audio=None) # Sanity check that value is in the library @@ -117,6 +122,7 @@ class LocalLibraryProviderTest(unittest.TestCase): self.assertEqual(result, self.tracks[0:1]) # Clear and refresh. + tmplib = os.path.join(tmpdir_local, 'library.json.gz') open(tmplib, 'w').close() backend.library.refresh() diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index bab70847..b99f8508 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -23,12 +23,11 @@ from tests.local import generate_song, populate_tracklist class LocalPlaybackProviderTest(unittest.TestCase): config = { 'core': { + 'data_dir': path_to_data_dir(''), 'max_tracklist_length': 10000, }, 'local': { 'media_dir': path_to_data_dir(''), - 'data_dir': path_to_data_dir(''), - 'playlists_dir': b'', 'library': 'json', } } diff --git a/tests/local/test_tracklist.py b/tests/local/test_tracklist.py index 72da3f13..b7ed7dcb 100644 --- a/tests/local/test_tracklist.py +++ b/tests/local/test_tracklist.py @@ -18,11 +18,11 @@ from tests.local import generate_song, populate_tracklist class LocalTracklistProviderTest(unittest.TestCase): config = { 'core': { + 'data_dir': path_to_data_dir(''), 'max_tracklist_length': 10000 }, 'local': { 'media_dir': path_to_data_dir(''), - 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', 'library': 'json', } From e1f349a6d432ea814be3adfa58c192f6e0df9b71 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Aug 2015 00:06:36 +0200 Subject: [PATCH 85/94] m3u: Make empty playlists_dir default to ext's data dir Partly fixes #1259 --- docs/changelog.rst | 6 ++++++ docs/ext/m3u.rst | 3 ++- mopidy/m3u/__init__.py | 2 +- mopidy/m3u/actor.py | 18 +++++++++++------- mopidy/m3u/ext.conf | 2 +- mopidy/m3u/playlists.py | 2 +- 6 files changed, 22 insertions(+), 11 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 9e1e5fec..b750be67 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -29,6 +29,12 @@ Bug fix release. :confval:`core/data_dir`, like the Debian and Arch packages. (Fixes: :issue:`1259`) +- M3U: Changed default for the :confval:`m3u/playlists_dir` from + ``$XDG_DATA_DIR/mopidy/m3u`` to unset, which now means the extension's data + dir. This does not change the defaults for desktop users, only system + services installed from packages that properly set :confval:`core/data_dir`, + like the Debian and Arch pakages. (Fixes: :issue:`1259`) + - Stream: If "file" is present in the :confval:`stream/protocols` config value and the :ref:`ext-file` extension is enabled, we exited with an error because two extensions claimed the same URI scheme. We now log a warning recommending diff --git a/docs/ext/m3u.rst b/docs/ext/m3u.rst index d05f88f1..2b86b73a 100644 --- a/docs/ext/m3u.rst +++ b/docs/ext/m3u.rst @@ -52,4 +52,5 @@ See :ref:`config` for general help on configuring Mopidy. .. confval:: m3u/playlists_dir - Path to directory with M3U files. + Path to directory with M3U files. Unset by default, in which case the + extension's data dir is used to store playlists. diff --git a/mopidy/m3u/__init__.py b/mopidy/m3u/__init__.py index e0fcf305..06825932 100644 --- a/mopidy/m3u/__init__.py +++ b/mopidy/m3u/__init__.py @@ -21,7 +21,7 @@ class Extension(ext.Extension): def get_config_schema(self): schema = super(Extension, self).get_config_schema() - schema['playlists_dir'] = config.Path() + schema['playlists_dir'] = config.Path(optional=True) return schema def setup(self, registry): diff --git a/mopidy/m3u/actor.py b/mopidy/m3u/actor.py index fe959d86..fc4734a2 100644 --- a/mopidy/m3u/actor.py +++ b/mopidy/m3u/actor.py @@ -4,7 +4,7 @@ import logging import pykka -from mopidy import backend +from mopidy import backend, m3u from mopidy.internal import encoding, path from mopidy.m3u.library import M3ULibraryProvider from mopidy.m3u.playlists import M3UPlaylistsProvider @@ -21,12 +21,16 @@ class M3UBackend(pykka.ThreadingActor, backend.Backend): self._config = config - try: - path.get_or_create_dir(config['m3u']['playlists_dir']) - except EnvironmentError as error: - logger.warning( - 'Could not create M3U playlists dir: %s', - encoding.locale_decode(error)) + if config['m3u']['playlists_dir'] is not None: + self._playlists_dir = config['m3u']['playlists_dir'] + try: + path.get_or_create_dir(self._playlists_dir) + except EnvironmentError as error: + logger.warning( + 'Could not create M3U playlists dir: %s', + encoding.locale_decode(error)) + else: + self._playlists_dir = m3u.Extension().get_data_dir(config) self.playlists = M3UPlaylistsProvider(backend=self) self.library = M3ULibraryProvider(backend=self) diff --git a/mopidy/m3u/ext.conf b/mopidy/m3u/ext.conf index 0e828b1b..adc0d00a 100644 --- a/mopidy/m3u/ext.conf +++ b/mopidy/m3u/ext.conf @@ -1,3 +1,3 @@ [m3u] enabled = true -playlists_dir = $XDG_DATA_DIR/mopidy/m3u +playlists_dir = diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index af92e062..3567f8aa 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -23,7 +23,7 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): def __init__(self, *args, **kwargs): super(M3UPlaylistsProvider, self).__init__(*args, **kwargs) - self._playlists_dir = self.backend._config['m3u']['playlists_dir'] + self._playlists_dir = self.backend._playlists_dir self._playlists = {} self.refresh() From 369f10b70676984f99a0f53926fe31ab5f329f10 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Aug 2015 00:24:49 +0200 Subject: [PATCH 86/94] docs: Link to PR #1266 --- docs/changelog.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index b750be67..741956b9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,19 +21,19 @@ Bug fix release. configs properly (e.g. Debian and Arch packages), this fix avoids the creation of a couple of directories that should not be used, typically :file:`/var/lib/mopidy/.local` and :file:`/var/lib/mopidy/.cache`. (Fixes: - :issue:`1259`) + :issue:`1259`, PR: :issue:`1266`) - Local: Deprecate :confval:`local/data_dir` and respect :confval:`core/data_dir` instead. This does not change the defaults for desktop users, only system services installed from packages that properly set :confval:`core/data_dir`, like the Debian and Arch packages. (Fixes: - :issue:`1259`) + :issue:`1259`, PR: :issue:`1266`) - M3U: Changed default for the :confval:`m3u/playlists_dir` from ``$XDG_DATA_DIR/mopidy/m3u`` to unset, which now means the extension's data dir. This does not change the defaults for desktop users, only system services installed from packages that properly set :confval:`core/data_dir`, - like the Debian and Arch pakages. (Fixes: :issue:`1259`) + like the Debian and Arch pakages. (Fixes: :issue:`1259`, PR: :issue:`1266`) - Stream: If "file" is present in the :confval:`stream/protocols` config value and the :ref:`ext-file` extension is enabled, we exited with an error because From d74f47ad58a7651dd45864c4c830d2f03bb4f859 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Aug 2015 17:33:51 +0200 Subject: [PATCH 87/94] main: Remove warnings if old settings dirs and files exists --- docs/changelog.rst | 15 +++++++++++++++ mopidy/__main__.py | 17 ----------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 741956b9..c999632b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,6 +5,21 @@ Changelog This changelog is used to track all major changes to Mopidy. +v1.2.0 (UNRELEASED) +=================== + +Feature release. + +Cleanups +-------- + +- Removed warning if :file:`~/.mopidy` exists. We stopped using this location + in 0.6, released in October 2011. + +- Removed warning if :file:`~/.config/mopidy/settings.py` exists. We stopped + using this settings file in 0.14, released in April 2013. + + v1.1.1 (UNRELEASED) =================== diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 208d2ff1..fbc750af 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -83,7 +83,6 @@ def main(): create_core_dirs(config) create_initial_config_file(args, extensions_data) - check_old_locations() verbosity_level = args.base_verbosity_level if args.verbosity_level: @@ -191,22 +190,6 @@ def create_initial_config_file(args, extensions_data): config_file, encoding.locale_decode(error)) -def check_old_locations(): - dot_mopidy_dir = path.expand_path(b'~/.mopidy') - if os.path.isdir(dot_mopidy_dir): - logger.warning( - 'Old Mopidy dot dir found at %s. Please migrate your config to ' - 'the ini-file based config format. See release notes for further ' - 'instructions.', dot_mopidy_dir) - - old_settings_file = path.expand_path(b'$XDG_CONFIG_DIR/mopidy/settings.py') - if os.path.isfile(old_settings_file): - logger.warning( - 'Old Mopidy settings file found at %s. Please migrate your ' - 'config to the ini-file based config format. See release notes ' - 'for further instructions.', old_settings_file) - - def log_extension_info(all_extensions, enabled_extensions): # TODO: distinguish disabled vs blocked by env? enabled_names = set(e.ext_name for e in enabled_extensions) From 4cae6ea64593a716097ef73e5765024e12648f56 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 27 Aug 2015 15:27:59 +0200 Subject: [PATCH 88/94] docs: ToC captions requires Sphinx 1.3 --- docs/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index cc760720..cbb2f228 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -78,7 +78,7 @@ def setup(app): # -- General configuration ---------------------------------------------------- -needs_sphinx = '1.0' +needs_sphinx = '1.3' extensions = [ 'sphinx.ext.autodoc', From 0360936135ef8193cc2b508cecb766e943b1294e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 28 Aug 2015 22:03:58 +0200 Subject: [PATCH 89/94] docs: Add a section on updating the local library Fixes #1267 --- docs/ext/local.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/ext/local.rst b/docs/ext/local.rst index d6e3b56a..ef9df5d7 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -35,6 +35,23 @@ To make a local library for your music available for Mopidy: #. Start Mopidy, find the music library in a client, and play some local music! +Updating the local library +========================== + +When you've added or removed music in your collection and want to update +Mopidy's index of your local library, you need to rescan:: + + mopidy local scan + +Note that if you are using the default local library storage, ``json``, you +need to restart Mopidy after the scan completes for the updated index to be +used. + +If you want index updates to come into effect immediately, you can try out +`Mopidy-Local-SQLite `_, which +will probably become the default backend in the near future. + + Pluggable library support ========================= From fcfb1515b13de427f173f3b91fffb0e4e84ee943 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 28 Aug 2015 22:20:52 +0200 Subject: [PATCH 90/94] local: Lower the default scan_flush_threshold @tkem recommends that this is reduced from 1000 to maximum 100 to not block incoming requests to Mopidy-Local-SQLite for too long while scanning the local library. --- docs/changelog.rst | 4 ++++ mopidy/local/ext.conf | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 741956b9..a507002e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -29,6 +29,10 @@ Bug fix release. :confval:`core/data_dir`, like the Debian and Arch packages. (Fixes: :issue:`1259`, PR: :issue:`1266`) +- Local: Change default value of :confval:`local/scan_flush_threshold` from + 1000 to 100 to shorten the time Mopidy-Local-SQLite blocks incoming requests + while scanning the local library. + - M3U: Changed default for the :confval:`m3u/playlists_dir` from ``$XDG_DATA_DIR/mopidy/m3u`` to unset, which now means the extension's data dir. This does not change the defaults for desktop users, only system diff --git a/mopidy/local/ext.conf b/mopidy/local/ext.conf index 6dd29147..b37a3a7a 100644 --- a/mopidy/local/ext.conf +++ b/mopidy/local/ext.conf @@ -3,7 +3,7 @@ enabled = true library = json media_dir = $XDG_MUSIC_DIR scan_timeout = 1000 -scan_flush_threshold = 1000 +scan_flush_threshold = 100 scan_follow_symlinks = false excluded_file_extensions = .directory From b480de77e1d7695ca74f345793376c3528c3b15e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 1 Sep 2015 15:32:21 +0200 Subject: [PATCH 91/94] core: Fix docstring error Fixes #1269 --- docs/changelog.rst | 5 ++++- mopidy/core/tracklist.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index a507002e..183dbf37 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -23,6 +23,9 @@ Bug fix release. :file:`/var/lib/mopidy/.local` and :file:`/var/lib/mopidy/.cache`. (Fixes: :issue:`1259`, PR: :issue:`1266`) +- Core: Fix error in :meth:`~mopidy.core.TracklistController.get_eot_tlid` + docstring. (Fixes: :issue:`1269`) + - Local: Deprecate :confval:`local/data_dir` and respect :confval:`core/data_dir` instead. This does not change the defaults for desktop users, only system services installed from packages that properly set @@ -126,7 +129,7 @@ Core API - Add ``tlid`` alternatives to methods that take ``tl_track`` and also add ``get_{eot,next,previous}_tlid`` methods as light weight alternatives to the - ``tl_track`` versions of the calls. (Fixes: :issue:`1131` PR: :issue:`1136`, + ``tl_track`` versions of the calls. (Fixes: :issue:`1131`, PR: :issue:`1136`, :issue:`1140`) - Add :meth:`mopidy.core.PlaybackController.get_current_tlid`. diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index db4e2a69..13efe322 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -236,7 +236,7 @@ class TracklistController(object): def get_eot_tlid(self): """ - The TLID of the track that will be played after the given track. + The TLID of the track that will be played after the current track. Not necessarily the same TLID as returned by :meth:`get_next_tlid`. @@ -332,7 +332,7 @@ class TracklistController(object): def get_previous_tlid(self): """ - Returns the TLID of the track that will be played if calling + Returns the TLID of the track that will be played if calling :meth:`mopidy.core.PlaybackController.previous()`. For normal playback this is the previous track in the tracklist. If From 715a989a5a07b4fd73d196a4748be32b6385fb01 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 1 Sep 2015 21:21:02 +0200 Subject: [PATCH 92/94] file: Allow lookup() of any file URI Fixes #1268 --- docs/changelog.rst | 10 ++++++++++ mopidy/file/library.py | 4 ---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 183dbf37..c66a738e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -63,6 +63,16 @@ Bug fix release. clients, which no longer scan the files twice. (Fixes: :issue:`1260`, PR: :issue:`1261`) +- File: Allow looking up metadata about any ``file://`` URI, just like we did + in Mopidy 1.0.x, where Mopidy-Stream handled ``file://`` URIs. In Mopidy + 1.1.0, Mopidy-File did not allow one to lookup files outside the directories + listed in :confval:`file/media_dir`. This broke Mopidy-Local-SQLite when the + :confval:`local/media_dir` directory was not within one of the + :confval:`file/media_dirs` directories. For browsing of files, we still limit + access to files inside the :confval:`file/media_dir` directories. For lookup, + you can now read metadata for any file you know the path of. (Fixes: + :issue:`1268`, PR: :issue:`1273`) + - Audio: Fix timeout handling in scanner. This regression caused timeouts to expire before it should, causing scans to fail. diff --git a/mopidy/file/library.py b/mopidy/file/library.py index b8531a6e..20ac0632 100644 --- a/mopidy/file/library.py +++ b/mopidy/file/library.py @@ -81,10 +81,6 @@ class FileLibraryProvider(backend.LibraryProvider): logger.debug('Looking up file URI: %s', uri) local_path = path.uri_to_path(uri) - if not self._is_in_basedir(local_path): - logger.warning('Ignoring URI outside base dir: %s', local_path) - return [] - try: result = self._scanner.scan(uri) track = utils.convert_tags_to_track(result.tags).copy( From 9957b3c2be965c7c5387230575eb7fe17efcdcf5 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 1 Sep 2015 23:27:39 +0200 Subject: [PATCH 93/94] local: Reintroduce local/data_dir config for the 1.1.x series As to not break compat with mopidy-local-* in v1.1.1. --- mopidy/local/__init__.py | 2 +- mopidy/local/ext.conf | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/mopidy/local/__init__.py b/mopidy/local/__init__.py index 3ee2703e..552e5341 100644 --- a/mopidy/local/__init__.py +++ b/mopidy/local/__init__.py @@ -23,7 +23,7 @@ class Extension(ext.Extension): schema = super(Extension, self).get_config_schema() schema['library'] = config.String() schema['media_dir'] = config.Path() - schema['data_dir'] = config.Deprecated() + schema['data_dir'] = config.Path(optional=True) schema['playlists_dir'] = config.Deprecated() schema['tag_cache_file'] = config.Deprecated() schema['scan_timeout'] = config.Integer( diff --git a/mopidy/local/ext.conf b/mopidy/local/ext.conf index b37a3a7a..c8fe6b86 100644 --- a/mopidy/local/ext.conf +++ b/mopidy/local/ext.conf @@ -2,6 +2,7 @@ enabled = true library = json media_dir = $XDG_MUSIC_DIR +data_dir = $XDG_DATA_DIR/mopidy/local scan_timeout = 1000 scan_flush_threshold = 100 scan_follow_symlinks = false From 8c2585771cedc809a96ae4971b23e1fabc9d9404 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 1 Sep 2015 23:31:41 +0200 Subject: [PATCH 94/94] local: Really deprecate local/data_dir --- docs/changelog.rst | 6 ++++++ mopidy/local/__init__.py | 2 +- mopidy/local/ext.conf | 1 - 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0856437c..9aef5d61 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,6 +10,12 @@ v1.2.0 (UNRELEASED) Feature release. +Local +----- + +- Made :confval:`local/data_dir` really deprecated. This change breaks older + versions of Mopidy-Local-SQLite and Mopidy-Local-Images. + Cleanups -------- diff --git a/mopidy/local/__init__.py b/mopidy/local/__init__.py index 552e5341..3ee2703e 100644 --- a/mopidy/local/__init__.py +++ b/mopidy/local/__init__.py @@ -23,7 +23,7 @@ class Extension(ext.Extension): schema = super(Extension, self).get_config_schema() schema['library'] = config.String() schema['media_dir'] = config.Path() - schema['data_dir'] = config.Path(optional=True) + schema['data_dir'] = config.Deprecated() schema['playlists_dir'] = config.Deprecated() schema['tag_cache_file'] = config.Deprecated() schema['scan_timeout'] = config.Integer( diff --git a/mopidy/local/ext.conf b/mopidy/local/ext.conf index c8fe6b86..b37a3a7a 100644 --- a/mopidy/local/ext.conf +++ b/mopidy/local/ext.conf @@ -2,7 +2,6 @@ enabled = true library = json media_dir = $XDG_MUSIC_DIR -data_dir = $XDG_DATA_DIR/mopidy/local scan_timeout = 1000 scan_flush_threshold = 100 scan_follow_symlinks = false