diff --git a/docs/changelog.rst b/docs/changelog.rst index 09f35c14..85bb28e2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -23,9 +23,13 @@ Core API - Add :meth:`mopidy.core.PlaylistsController.get_uri_schemes`. (PR: :issue:`1362`) -- Persist state between runs. The amount of data to persist can be +- Persist state between runs. The amount of data to persist can be controlled by config value :confval:`core/restore_state` +- The ``track_playback_ended`` event now includes the correct ``tl_track`` + reference when changing to the next track in consume mode. (Fixes: + :issue:`1402` PR: :issue:`1403` PR: :issue:`1406`) + Models ------ @@ -40,7 +44,7 @@ Extension support we let Mopidy crash if an extension's setup crashed. (PR: :issue:`1337`) Local backend --------------- +------------- - Made :confval:`local/data_dir` really deprecated. This change breaks older versions of Mopidy-Local-SQLite and Mopidy-Local-Images. @@ -51,6 +55,23 @@ M3U backend - Derive track name from file name for non-extended M3U playlists. (Fixes: :issue:`1364`, PR: :issue:`1369`) +- Major refactoring of the M3U playlist extension. (Fixes: + :issue:`1370` PR: :issue:`1386`) + + - Add :confval:`m3u/default_encoding` and :confval:`m3u/default_extension` + config values for improved text encoding support. + + - No longer scan playlist directory and parse playlists at startup or refresh. + Similarly to the file extension, this now happens on request. + + - Use :class:`mopidy.models.Ref` instances when reading and writing + playlists. Therefore, ``Track.length`` is no longer stored in + extended M3U playlists and ``#EXTINF`` runtime is always set to + -1. + + - Improve reliability of playlist updates using the core playlist API by + applying the write-replace pattern for file updates. + MPD frontend ------------ diff --git a/docs/ext/m3u.rst b/docs/ext/m3u.rst index 2b86b73a..37dc60be 100644 --- a/docs/ext/m3u.rst +++ b/docs/ext/m3u.rst @@ -54,3 +54,14 @@ See :ref:`config` for general help on configuring Mopidy. Path to directory with M3U files. Unset by default, in which case the extension's data dir is used to store playlists. + +.. confval:: m3u/default_encoding + + Text encoding used for files with extension ``.m3u``. Default is + ``latin-1``. Note that files with extension ``.m3u8`` are always + expected to be UTF-8 encoded. + +.. confval:: m3u/default_extension + + The file extension for M3U playlists created using the core playlist + API. Default is ``.m3u8``. diff --git a/docs/extensiondev.rst b/docs/extensiondev.rst index f797368f..72747e28 100644 --- a/docs/extensiondev.rst +++ b/docs/extensiondev.rst @@ -542,3 +542,245 @@ your HTTP requests:: For further details, see Requests' docs on `session objects `__. + +Testing extensions +================== + +Creating test cases for your extensions makes them much simpler to maintain +over the long term. It can also make it easier for you to review and accept +pull requests from other contributors knowing that they will not break the +extension in some unanticipated way. + +Before getting started, it is important to familiarize yourself with the +Python `mock library `_. +When it comes to running tests, Mopidy typically makes use of testing tools +like `tox `_ and +`pytest `_. + +Testing approach +---------------- + +To a large extent the testing approach to follow depends on how your extension +is structured, which parts of Mopidy it interacts with, and if it uses any 3rd +party APIs or makes any HTTP requests to the outside world. + +The sections that follow contain code extracts that highlight some of the +key areas that should be tested. For more exhaustive examples, you may want to +take a look at the test cases that ship with Mopidy itself which covers +everything from instantiating various controllers, reading configuration files, +and simulating events that your extension can listen to. + +In general your tests should cover the extension definition, the relevant +Mopidy controllers, and the Pykka backend and / or frontend actors that form +part of the extension. + +Testing the extension definition +-------------------------------- + +Test cases for checking the definition of the extension should ensure that: + +- the extension provides a ``ext.conf`` configuration file containing the + relevant parameters with their default values, +- that the config schema is fully defined, and +- that the extension's actor(s) are added to the Mopidy registry on setup. + +An example of what these tests could look like is provided below:: + + def test_get_default_config(self): + ext = Extension() + config = ext.get_default_config() + + assert '[my_extension]' in config + assert 'enabled = true' in config + assert 'param_1 = value_1' in config + assert 'param_2 = value_2' in config + assert 'param_n = value_n' in config + + def test_get_config_schema(self): + ext = Extension() + schema = ext.get_config_schema() + + assert 'enabled' in schema + assert 'param_1' in schema + assert 'param_2' in schema + assert 'param_n' in schema + + def test_setup(self): + registry = mock.Mock() + + ext = Extension() + ext.setup(registry) + calls = [mock.call('frontend', frontend_lib.MyFrontend), + mock.call('backend', backend_lib.MyBackend)] + registry.add.assert_has_calls(calls, any_order=True) + + +Testing backend actors +---------------------- + +Backends can usually be constructed with a small mockup of the configuration +file, and mocking the audio actor:: + + @pytest.fixture + def config(): + return { + 'http': { + 'hostname': '127.0.0.1', + 'port': '6680' + }, + 'proxy': { + 'hostname': 'host_mock', + 'port': 'port_mock' + }, + 'my_extension': { + 'enabled': True, + 'param_1': 'value_1', + 'param_2': 'value_2', + 'param_n': 'value_n', + } + } + + def get_backend(config): + return backend.MyBackend(config=config, audio=mock.Mock()) + +The following libraries might be useful for mocking any HTTP requests that +your extension makes: + +- `responses `_ - A utility library for + mocking out the requests Python library. +- `vcrpy `_ - Automatically mock your HTTP + interactions to simplify and speed up testing. + +At the very least, you'll probably want to patch ``requests`` or any other web +API's that you use to avoid any unintended HTTP requests from being made by +your backend during testing:: + + from mock import patch + @mock.patch('requests.get', + mock.Mock(side_effect=Exception('Intercepted unintended HTTP call'))) + + +Backend tests should also ensure that: + +- the backend provides a unique URI scheme, +- that it sets up the various providers (e.g. library, playback, etc.) + +:: + + def test_uri_schemes(config): + backend = get_backend(config) + + assert 'my_scheme' in backend.uri_schemes + + + def test_init_sets_up_the_providers(config): + backend = get_backend(config) + + assert isinstance(backend.library, library.MyLibraryProvider) + assert isinstance(backend.playback, playback.MyPlaybackProvider) + + +Once you have a backend instance to work with, testing the various playback, +library, and other providers is straight forward and should not require any +special setup or processing. + +Testing libraries +----------------- + +Library test cases should cover the implementations of the standard Mopidy +API (e.g. ``browse``, ``lookup``, ``refresh``, ``get_images``, ``search``, +etc.) + +Testing playback controllers +---------------------------- + +Testing ``change_track`` and ``translate_uri`` is probably the highest +priority, since these methods are used to prepare the track and provide its +audio URL to Mopidy's core for playback. + +Testing frontends +----------------- + +Because most frontends will interact with the Mopidy core, it will most likely +be necessary to have a full core running for testing purposes:: + + self.core = core.Core.start( + config, backends=[get_backend(config)]).proxy() + + +It may be advisable to take a quick look at the +`Pykka API `_ at this point to make sure that +you are familiar with ``ThreadingActor``, ``ThreadingFuture``, and the +``proxies`` that allow you to access the attributes and methods of the actor +directly. + +You'll also need a list of :class:`~mopidy.models.Track` and a list of URIs in +order to populate the core with some simple tracks that can be used for +testing:: + + class BaseTest(unittest.TestCase): + tracks = [ + models.Track(uri='my_scheme:track:id1', length=40000), # Regular track + models.Track(uri='my_scheme:track:id2', length=None), # No duration + ] + + uris = [ 'my_scheme:track:id1', 'my_scheme:track:id2'] + + +In the ``setup()`` method of your test class, you will then probably need to +monkey patch looking up tracks in the library (so that it will always use the +lists that you defined), and then populate the core's tracklist:: + + def lookup(uris): + result = {uri: [] for uri in uris} + for track in self.tracks: + if track.uri in result: + result[track.uri].append(track) + return result + + self.core.library.lookup = lookup + self.tl_tracks = self.core.tracklist.add(uris=self.uris).get() + + +With all of that done you should finally be ready to instantiate your frontend:: + + self.frontend = frontend.MyFrontend.start(config(), self.core).proxy() + + +Keep in mind that the normal core and frontend methods will usually return +``pykka.ThreadingFuture`` objects, so you will need to add ``.get()`` at +the end of most method calls in order to get to the actual return values. + +Triggering events +----------------- + +There may be test case scenarios that require simulating certain event triggers +that your extension's actors can listen for and respond on. An example for +patching the listener to store these events, and then play them back for your +actor, may look something like this:: + + self.events = [] + self.patcher = mock.patch('mopidy.listener.send') + self.send_mock = self.patcher.start() + + def send(cls, event, **kwargs): + self.events.append((event, kwargs)) + + self.send_mock.side_effect = send + + +Once all of the events have been captured, a method like +``replay_events()`` can be called at the relevant points in the code to have +the events fire:: + + def replay_events(self, my_actor, until=None): + while self.events: + if self.events[0][0] == until: + break + event, kwargs = self.events.pop(0) + frontend.on_event(event, **kwargs).get() + + +For further details and examples, refer to the +`/tests `_ +directory on the Mopidy development branch. \ No newline at end of file diff --git a/mopidy/backend.py b/mopidy/backend.py index 8616ae96..7412ccc6 100644 --- a/mopidy/backend.py +++ b/mopidy/backend.py @@ -347,13 +347,14 @@ class PlaylistsProvider(object): """ Create a new empty playlist with the given name. - Returns a new playlist with the given name and an URI. + Returns a new playlist with the given name and an URI, or :class:`None` + on failure. *MUST be implemented by subclass.* :param name: name of the new playlist :type name: string - :rtype: :class:`mopidy.models.Playlist` + :rtype: :class:`mopidy.models.Playlist` or :class:`None` """ raise NotImplementedError diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 54d30230..cb89658a 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -515,7 +515,8 @@ class PlaybackController(object): listener.CoreListener.send('track_playback_started', tl_track=tl_track) def _trigger_track_playback_ended(self, time_position_before_stop): - if self.get_current_tl_track() is None: + tl_track = self.get_current_tl_track() + if tl_track is None: return logger.debug('Triggering track playback ended event') @@ -527,7 +528,7 @@ class PlaybackController(object): # TODO: Use the lowest of track duration and position. listener.CoreListener.send( 'track_playback_ended', - tl_track=self.get_current_tl_track(), + tl_track=tl_track, time_position=time_position_before_stop) def _trigger_playback_state_changed(self, old_state, new_state): diff --git a/mopidy/m3u/__init__.py b/mopidy/m3u/__init__.py index 06825932..df769c88 100644 --- a/mopidy/m3u/__init__.py +++ b/mopidy/m3u/__init__.py @@ -21,10 +21,11 @@ class Extension(ext.Extension): def get_config_schema(self): schema = super(Extension, self).get_config_schema() + schema['default_encoding'] = config.String() + schema['default_extension'] = config.String(choices=['.m3u', '.m3u8']) schema['playlists_dir'] = config.Path(optional=True) return schema def setup(self, registry): - from .actor import M3UBackend - + from .backend import M3UBackend registry.add('backend', M3UBackend) diff --git a/mopidy/m3u/actor.py b/mopidy/m3u/actor.py deleted file mode 100644 index 55257f87..00000000 --- a/mopidy/m3u/actor.py +++ /dev/null @@ -1,36 +0,0 @@ -from __future__ import absolute_import, unicode_literals - -import logging - -import pykka - -from mopidy import backend, m3u -from mopidy.internal import encoding, path -from mopidy.m3u.library import M3ULibraryProvider -from mopidy.m3u.playlists import M3UPlaylistsProvider - - -logger = logging.getLogger(__name__) - - -class M3UBackend(pykka.ThreadingActor, backend.Backend): - uri_schemes = ['m3u'] - - def __init__(self, config, audio): - super(M3UBackend, self).__init__() - - self._config = config - - 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/backend.py b/mopidy/m3u/backend.py new file mode 100644 index 00000000..02719cc7 --- /dev/null +++ b/mopidy/m3u/backend.py @@ -0,0 +1,15 @@ +from __future__ import absolute_import, unicode_literals + +import pykka + +from mopidy import backend + +from . import playlists + + +class M3UBackend(pykka.ThreadingActor, backend.Backend): + uri_schemes = ['m3u'] + + def __init__(self, config, audio): + super(M3UBackend, self).__init__() + self.playlists = playlists.M3UPlaylistsProvider(self, config) diff --git a/mopidy/m3u/ext.conf b/mopidy/m3u/ext.conf index adc0d00a..862bc6f7 100644 --- a/mopidy/m3u/ext.conf +++ b/mopidy/m3u/ext.conf @@ -1,3 +1,5 @@ [m3u] enabled = true playlists_dir = +default_encoding = latin-1 +default_extension = .m3u8 diff --git a/mopidy/m3u/library.py b/mopidy/m3u/library.py deleted file mode 100644 index 291a6194..00000000 --- a/mopidy/m3u/library.py +++ /dev/null @@ -1,19 +0,0 @@ -from __future__ import absolute_import, unicode_literals - -import logging - -from mopidy import backend - -logger = logging.getLogger(__name__) - - -class M3ULibraryProvider(backend.LibraryProvider): - - """Library for looking up M3U playlists.""" - - def __init__(self, backend): - super(M3ULibraryProvider, self).__init__(backend) - - def lookup(self, uri): - # TODO Lookup tracks in M3U playlist - return [] diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index 3567f8aa..7e4e39ff 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -1,117 +1,149 @@ -from __future__ import absolute_import, division, unicode_literals +from __future__ import absolute_import, unicode_literals -import glob +import contextlib +import io +import locale import logging import operator import os -import re -import sys +import tempfile from mopidy import backend -from mopidy.m3u import translator -from mopidy.models import Playlist, Ref +from . import Extension, translator logger = logging.getLogger(__name__) +def log_environment_error(message, error): + if isinstance(error.strerror, bytes): + strerror = error.strerror.decode(locale.getpreferredencoding()) + else: + strerror = error.strerror + logger.error('%s: %s', message, strerror) + + +@contextlib.contextmanager +def replace(path, mode='w+b', encoding=None, errors=None): + try: + (fd, tempname) = tempfile.mkstemp(dir=os.path.dirname(path)) + except TypeError: + # Python 3 requires dir to be of type str until v3.5 + import sys + path = path.decode(sys.getfilesystemencoding()) + (fd, tempname) = tempfile.mkstemp(dir=os.path.dirname(path)) + try: + fp = io.open(fd, mode, encoding=encoding, errors=errors) + except: + os.remove(tempname) + os.close(fd) + raise + try: + yield fp + fp.flush() + os.fsync(fd) + os.rename(tempname, path) + except: + os.remove(tempname) + raise + finally: + fp.close() + + class M3UPlaylistsProvider(backend.PlaylistsProvider): - # TODO: currently this only handles UNIX file systems - _invalid_filename_chars = re.compile(r'[/]') + def __init__(self, backend, config): + super(M3UPlaylistsProvider, self).__init__(backend) - def __init__(self, *args, **kwargs): - super(M3UPlaylistsProvider, self).__init__(*args, **kwargs) - - self._playlists_dir = self.backend._playlists_dir - self._playlists = {} - self.refresh() + ext_config = config[Extension.ext_name] + if ext_config['playlists_dir'] is None: + self._playlists_dir = Extension.get_data_dir(config) + else: + self._playlists_dir = ext_config['playlists_dir'] + self._default_encoding = ext_config['default_encoding'] + self._default_extension = ext_config['default_extension'] def as_list(self): - refs = [ - Ref.playlist(uri=pl.uri, name=pl.name) - for pl in self._playlists.values()] - return sorted(refs, key=operator.attrgetter('name')) - - def get_items(self, uri): - playlist = self._playlists.get(uri) - if playlist is None: - return None - return [Ref.track(uri=t.uri, name=t.name) for t in playlist.tracks] + result = [] + for entry in os.listdir(self._playlists_dir): + if not entry.endswith((b'.m3u', b'.m3u8')): + continue + elif not os.path.isfile(self._abspath(entry)): + continue + else: + result.append(translator.path_to_ref(entry)) + result.sort(key=operator.attrgetter('name')) + return result def create(self, name): - playlist = self._save_m3u(Playlist(name=name)) - self._playlists[playlist.uri] = playlist - logger.info('Created playlist %s', playlist.uri) - return playlist + path = translator.path_from_name(name.strip(), self._default_extension) + try: + with self._open(path, 'w'): + pass + mtime = os.path.getmtime(self._abspath(path)) + except EnvironmentError as e: + log_environment_error('Error creating playlist %s' % name, e) + else: + return translator.playlist(path, [], mtime) def delete(self, uri): - if uri in self._playlists: - path = translator.playlist_uri_to_path(uri, self._playlists_dir) - if os.path.exists(path): - os.remove(path) - else: - logger.warning( - 'Trying to delete missing playlist file %s', path) - del self._playlists[uri] - logger.info('Deleted playlist %s', uri) + path = translator.uri_to_path(uri) + try: + os.remove(self._abspath(path)) + except EnvironmentError as e: + log_environment_error('Error deleting playlist %s' % uri, e) + + def get_items(self, uri): + path = translator.uri_to_path(uri) + try: + with self._open(path, 'r') as fp: + items = translator.load_items(fp, self._playlists_dir) + except EnvironmentError as e: + log_environment_error('Error reading playlist %s' % uri, e) else: - logger.warning('Trying to delete unknown playlist %s', uri) + return items def lookup(self, uri): - return self._playlists.get(uri) + path = translator.uri_to_path(uri) + try: + with self._open(path, 'r') as fp: + items = translator.load_items(fp, self._playlists_dir) + mtime = os.path.getmtime(self._abspath(path)) + except EnvironmentError as e: + log_environment_error('Error reading playlist %s' % uri, e) + else: + return translator.playlist(path, items, mtime) def refresh(self): - playlists = {} - - encoding = sys.getfilesystemencoding() - 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') - tracks = translator.parse_m3u(path) - playlists[uri] = Playlist(uri=uri, name=name, tracks=tracks) - - self._playlists = playlists - - logger.info( - 'Loaded %d M3U playlists from %s', - len(playlists), self._playlists_dir) - - # TODO Trigger playlists_loaded event? + pass # nothing to do def save(self, playlist): - assert playlist.uri, 'Cannot save playlist without URI' - assert playlist.uri in self._playlists, \ - 'Cannot save playlist with unknown URI: %s' % playlist.uri - - original_uri = playlist.uri - playlist = self._save_m3u(playlist) - if playlist.uri != original_uri and original_uri in self._playlists: - self.delete(original_uri) - self._playlists[playlist.uri] = playlist - return playlist - - def _sanitize_m3u_name(self, name, encoding=sys.getfilesystemencoding()): - name = self._invalid_filename_chars.sub('|', name.strip()) - # make sure we end up with a valid path segment - name = name.encode(encoding, errors='replace') - name = os.path.basename(name) # paranoia? - name = name.decode(encoding) - return name - - def _save_m3u(self, playlist, encoding=sys.getfilesystemencoding()): - if playlist.name: - name = self._sanitize_m3u_name(playlist.name, encoding) - uri = translator.path_to_playlist_uri( - name.encode(encoding) + b'.m3u') - path = translator.playlist_uri_to_path(uri, self._playlists_dir) - elif playlist.uri: - uri = playlist.uri - path = translator.playlist_uri_to_path(uri, self._playlists_dir) - name, _ = os.path.splitext(os.path.basename(path).decode(encoding)) + path = translator.uri_to_path(playlist.uri) + name = translator.name_from_path(path) + try: + with self._open(path, 'w') as fp: + translator.dump_items(playlist.tracks, fp) + if playlist.name and playlist.name != name: + opath, ext = os.path.splitext(path) + path = translator.path_from_name(playlist.name.strip()) + ext + os.rename(self._abspath(opath + ext), self._abspath(path)) + mtime = os.path.getmtime(self._abspath(path)) + except EnvironmentError as e: + log_environment_error('Error saving playlist %s' % playlist.uri, e) else: - raise ValueError('M3U playlist needs name or URI') - translator.save_m3u(path, playlist.tracks, 'latin1') - # assert playlist name matches file name/uri - return playlist.replace(uri=uri, name=name) + return translator.playlist(path, playlist.tracks, mtime) + + def _abspath(self, path): + return os.path.join(self._playlists_dir, path) + + def _open(self, path, mode='r'): + if path.endswith(b'.m3u8'): + encoding = 'utf-8' + else: + encoding = self._default_encoding + if not os.path.isabs(path): + path = os.path.join(self._playlists_dir, path) + if 'w' in mode: + return replace(path, mode, encoding=encoding, errors='replace') + else: + return io.open(path, mode, encoding=encoding, errors='replace') diff --git a/mopidy/m3u/translator.py b/mopidy/m3u/translator.py index 764cf84b..da74cc1b 100644 --- a/mopidy/m3u/translator.py +++ b/mopidy/m3u/translator.py @@ -1,130 +1,119 @@ -from __future__ import absolute_import, unicode_literals +from __future__ import absolute_import, print_function, unicode_literals -import codecs -import logging import os -import re -from mopidy import compat -from mopidy.compat import urllib -from mopidy.internal import encoding, path -from mopidy.models import Track +from mopidy import models + +from . import Extension + +try: + from urllib.parse import quote_from_bytes, unquote_to_bytes +except ImportError: + import urllib + + def quote_from_bytes(bytes, safe=b'/'): + # Python 3 returns Unicode string + return urllib.quote(bytes, safe).decode('utf-8') + + def unquote_to_bytes(string): + if isinstance(string, bytes): + return urllib.unquote(string) + else: + return urllib.unquote(string.encode('utf-8')) + +try: + from urllib.parse import urlsplit, urlunsplit +except ImportError: + from urlparse import urlsplit, urlunsplit -M3U_EXTINF_RE = re.compile(r'#EXTINF:(-1|\d+),(.*)') +try: + from os import fsencode, fsdecode +except ImportError: + import sys -logger = logging.getLogger(__name__) + # no 'surrogateescape' in Python 2; 'replace' for backward compatibility + def fsencode(filename, encoding=sys.getfilesystemencoding()): + return filename.encode(encoding, 'replace') + + def fsdecode(filename, encoding=sys.getfilesystemencoding()): + return filename.decode(encoding, 'replace') -def playlist_uri_to_path(uri, playlists_dir): - if not uri.startswith('m3u:'): - raise ValueError('Invalid URI %s' % uri) - file_path = path.uri_to_path(uri) - return os.path.join(playlists_dir, file_path) +def path_to_uri(path, scheme=Extension.ext_name): + """Convert file path to URI.""" + assert isinstance(path, bytes), 'Mopidy paths should be bytes' + uripath = quote_from_bytes(os.path.normpath(path)) + return urlunsplit((scheme, None, uripath, None, None)) -def path_to_playlist_uri(relpath): - """Convert path relative to playlists_dir to M3U URI.""" - if isinstance(relpath, compat.text_type): - relpath = relpath.encode('utf-8') - return b'm3u:%s' % urllib.parse.quote(relpath) +def uri_to_path(uri): + """Convert URI to file path.""" + # TODO: decide on Unicode vs. bytes for URIs + return unquote_to_bytes(urlsplit(uri).path) -def m3u_extinf_to_track(line): - """Convert extended M3U directive to track template.""" - m = M3U_EXTINF_RE.match(line) - if not m: - logger.warning('Invalid extended M3U directive: %s', line) - return Track() - (runtime, title) = m.groups() - if int(runtime) > 0: - return Track(name=title, length=1000 * int(runtime)) - else: - return Track(name=title) - - -def parse_m3u(file_path, media_dir=None): - r""" - Convert M3U file list to list of tracks - - Example M3U data:: - - # This is a comment - Alternative\Band - Song.mp3 - Classical\Other Band - New Song.mp3 - Stuff.mp3 - D:\More Music\Foo.mp3 - http://www.example.com:8000/Listen.pls - http://www.example.com/~user/Mine.mp3 - - Example extended M3U data:: - - #EXTM3U - #EXTINF:123, Sample artist - Sample title - Sample.mp3 - #EXTINF:321,Example Artist - Example title - Greatest Hits\Example.ogg - #EXTINF:-1,Radio XMP - http://mp3stream.example.com:8000/ - - - Relative paths of songs should be with respect to location of M3U. - - Paths are normally platform specific. - - 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 = [] +def name_from_path(path): + """Extract name from file path.""" + name, _ = os.path.splitext(os.path.basename(path)) try: - 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', encoding.locale_decode(error)) - return tracks + return fsdecode(name) + except UnicodeError: + return None - if not contents: - return tracks - # Strip newlines left by codecs - contents = [line.strip() for line in contents] +def path_from_name(name, ext=None, sep='|'): + """Convert name with optional extension to file path.""" + if ext: + return fsencode(name.replace(os.sep, sep) + ext) + else: + return fsencode(name.replace(os.sep, sep)) - extended = contents[0].startswith('#EXTM3U') - track = Track() - for line in contents: +def path_to_ref(path): + return models.Ref.playlist( + uri=path_to_uri(path), + name=name_from_path(path) + ) + + +def load_items(fp, basedir): + refs = [] + name = None + for line in filter(None, (line.strip() for line in fp)): if line.startswith('#'): - if extended and line.startswith('#EXTINF'): - track = m3u_extinf_to_track(line) + if line.startswith('#EXTINF:'): + name = line.partition(',')[2] continue - if not track.name: - name = os.path.basename(os.path.splitext(line)[0]) - track = track.replace(name=urllib.parse.unquote(name)) - if urllib.parse.urlsplit(line).scheme: - tracks.append(track.replace(uri=line)) - elif os.path.normpath(line) == os.path.abspath(line): - uri = path.path_to_uri(line) - tracks.append(track.replace(uri=uri)) - elif media_dir is not None: - uri = path.path_to_uri(os.path.join(media_dir, line)) - tracks.append(track.replace(uri=uri)) - - track = Track() - return tracks + elif not urlsplit(line).scheme: + path = os.path.join(basedir, fsencode(line)) + if not name: + name = name_from_path(path) + uri = path_to_uri(path, scheme='file') + else: + uri = line # do *not* extract name from (stream?) URI path + refs.append(models.Ref.track(uri=uri, name=name)) + name = None + return refs -def save_m3u(filename, tracks, encoding='latin1', errors='replace'): - extended = any(track.name for track in tracks) - # codecs.open() always uses binary mode, just being explicit here - with codecs.open(filename, 'wb', encoding, errors) as m3u: - if extended: - m3u.write('#EXTM3U' + os.linesep) - for track in tracks: - if extended and track.name: - m3u.write('#EXTINF:%d,%s%s' % ( - track.length // 1000 if track.length else -1, - track.name, - os.linesep)) - m3u.write(track.uri + os.linesep) +def dump_items(items, fp): + if any(item.name for item in items): + print('#EXTM3U', file=fp) + for item in items: + if item.name: + print('#EXTINF:-1,%s' % item.name, file=fp) + # TODO: convert file URIs to (relative) paths? + if isinstance(item.uri, bytes): + print(item.uri.decode('utf-8'), file=fp) + else: + print(item.uri, file=fp) + + +def playlist(path, items=[], mtime=None): + return models.Playlist( + uri=path_to_uri(path), + name=name_from_path(path), + tracks=[models.Track(uri=item.uri, name=item.name) for item in items], + last_modified=(int(mtime * 1000) if mtime else None) + ) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 6d66bca6..96528983 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -492,6 +492,36 @@ class EventEmissionTest(BaseTest): ], listener_mock.send.mock_calls) + def test_next_emits_events_when_consume_mode_is_enabled( + self, + listener_mock): + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.tracklist.set_consume(True) + self.core.playback.play(tl_tracks[0]) + self.replay_events() + self.core.playback.seek(1000) + self.replay_events() + listener_mock.reset_mock() + + self.core.playback.next() + self.replay_events() + + self.assertListEqual( + [ + mock.call( + 'tracklist_changed'), + mock.call( + 'track_playback_ended', + tl_track=tl_tracks[0], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='playing', new_state='playing'), + mock.call( + 'track_playback_started', tl_track=tl_tracks[1]), + ], + listener_mock.send.mock_calls) + def test_gapless_track_change_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index edebe65b..664da9e9 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -7,14 +7,12 @@ import platform import shutil import tempfile import unittest -import urllib import pykka from mopidy import core from mopidy.internal import deprecation -from mopidy.m3u import actor -from mopidy.m3u.translator import playlist_uri_to_path +from mopidy.m3u.backend import M3UBackend from mopidy.models import Playlist, Track from tests import dummy_audio, path_to_data_dir @@ -22,9 +20,12 @@ from tests.m3u import generate_song class M3UPlaylistsProviderTest(unittest.TestCase): - backend_class = actor.M3UBackend + backend_class = M3UBackend config = { 'm3u': { + 'enabled': True, + 'default_encoding': 'latin-1', + 'default_extension': '.m3u', 'playlists_dir': path_to_data_dir(''), } } @@ -34,7 +35,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.playlists_dir = self.config['m3u']['playlists_dir'] audio = dummy_audio.create_proxy() - backend = actor.M3UBackend.start( + backend = M3UBackend.start( config=self.config, audio=audio).proxy() self.core = core.Core(backends=[backend]) @@ -46,7 +47,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_created_playlist_is_persisted(self): uri = 'm3u:test.m3u' - path = playlist_uri_to_path(uri, self.playlists_dir) + path = os.path.join(self.playlists_dir, b'test.m3u') self.assertFalse(os.path.exists(path)) playlist = self.core.playlists.create('test') @@ -57,7 +58,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_create_sanitizes_playlist_name(self): playlist = self.core.playlists.create(' ../../test FOO baR ') self.assertEqual('..|..|test FOO baR', playlist.name) - path = playlist_uri_to_path(playlist.uri, self.playlists_dir) + path = os.path.join(self.playlists_dir, b'..|..|test FOO baR.m3u') self.assertEqual(self.playlists_dir, os.path.dirname(path)) self.assertTrue(os.path.exists(path)) @@ -65,8 +66,8 @@ class M3UPlaylistsProviderTest(unittest.TestCase): uri1 = 'm3u:test1.m3u' uri2 = 'm3u:test2.m3u' - path1 = playlist_uri_to_path(uri1, self.playlists_dir) - path2 = playlist_uri_to_path(uri2, self.playlists_dir) + path1 = os.path.join(self.playlists_dir, b'test1.m3u') + path2 = os.path.join(self.playlists_dir, b'test2.m3u') playlist = self.core.playlists.create('test1') self.assertEqual('test1', playlist.name) @@ -82,7 +83,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_deleted_playlist_is_removed(self): uri = 'm3u:test.m3u' - path = playlist_uri_to_path(uri, self.playlists_dir) + path = os.path.join(self.playlists_dir, b'test.m3u') self.assertFalse(os.path.exists(path)) @@ -98,7 +99,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): track = Track(uri=generate_song(1)) playlist = self.core.playlists.create('test') playlist = self.core.playlists.save(playlist.replace(tracks=[track])) - path = playlist_uri_to_path(playlist.uri, self.playlists_dir) + path = os.path.join(self.playlists_dir, b'test.m3u') with open(path) as f: contents = f.read() @@ -109,32 +110,32 @@ class M3UPlaylistsProviderTest(unittest.TestCase): track = Track(uri=generate_song(1), name='Test', length=60000) playlist = self.core.playlists.create('test') playlist = self.core.playlists.save(playlist.replace(tracks=[track])) - path = playlist_uri_to_path(playlist.uri, self.playlists_dir) + path = os.path.join(self.playlists_dir, b'test.m3u') with open(path) as f: m3u = f.read().splitlines() - self.assertEqual(['#EXTM3U', '#EXTINF:60,Test', track.uri], m3u) + self.assertEqual(['#EXTM3U', '#EXTINF:-1,Test', track.uri], m3u) def test_latin1_playlist_contents_is_written_to_disk(self): track = Track(uri=generate_song(1), name='Test\x9f', length=60000) playlist = self.core.playlists.create('test') playlist = self.core.playlists.save(playlist.copy(tracks=[track])) - path = playlist_uri_to_path(playlist.uri, self.playlists_dir) + path = os.path.join(self.playlists_dir, b'test.m3u') with open(path, 'rb') as f: m3u = f.read().splitlines() - self.assertEqual([b'#EXTM3U', b'#EXTINF:60,Test\x9f', track.uri], m3u) + self.assertEqual([b'#EXTM3U', b'#EXTINF:-1,Test\x9f', track.uri], m3u) def test_utf8_playlist_contents_is_replaced_and_written_to_disk(self): track = Track(uri=generate_song(1), name='Test\u07b4', length=60000) playlist = self.core.playlists.create('test') playlist = self.core.playlists.save(playlist.copy(tracks=[track])) - path = playlist_uri_to_path(playlist.uri, self.playlists_dir) + path = os.path.join(self.playlists_dir, b'test.m3u') with open(path, 'rb') as f: m3u = f.read().splitlines() - self.assertEqual([b'#EXTM3U', b'#EXTINF:60,Test?', track.uri], m3u) + self.assertEqual([b'#EXTM3U', b'#EXTINF:-1,Test?', track.uri], m3u) def test_playlists_are_loaded_at_startup(self): track = Track(uri='dummy:track:path2') @@ -149,8 +150,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.assertEqual(track.uri, result.tracks[0].uri) def test_load_playlist_with_nonfilesystem_encoding_of_filename(self): - uri = 'm3u:%s.m3u' % urllib.quote('øæå'.encode('latin-1')) - path = playlist_uri_to_path(uri, self.playlists_dir) + path = os.path.join(self.playlists_dir, 'øæå.m3u'.encode('latin-1')) with open(path, 'wb+') as f: f.write(b'#EXTM3U\n') @@ -198,7 +198,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): playlist = self.core.playlists.create('test') self.assertEqual(playlist, self.core.playlists.lookup(playlist.uri)) - path = playlist_uri_to_path(playlist.uri, self.playlists_dir) + path = os.path.join(self.playlists_dir, b'test.m3u') self.assertTrue(os.path.exists(path)) os.remove(path) @@ -245,12 +245,9 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_save_playlist_with_new_uri(self): uri = 'm3u:test.m3u' - - with self.assertRaises(AssertionError): - self.core.playlists.save(Playlist(uri=uri)) - - path = playlist_uri_to_path(uri, self.playlists_dir) - self.assertFalse(os.path.exists(path)) + self.core.playlists.save(Playlist(uri=uri)) + path = os.path.join(self.playlists_dir, b'test.m3u') + self.assertTrue(os.path.exists(path)) def test_playlist_with_unknown_track(self): track = Track(uri='file:///dev/null') diff --git a/tests/m3u/test_translator.py b/tests/m3u/test_translator.py index 88387cb3..35efed4c 100644 --- a/tests/m3u/test_translator.py +++ b/tests/m3u/test_translator.py @@ -2,137 +2,145 @@ from __future__ import absolute_import, unicode_literals -import os -import tempfile -import unittest +import io -from mopidy.internal import path from mopidy.m3u import translator -from mopidy.models import Track - -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) -song4_uri = 'http://example.com/foo%20bar.mp3' -encoded_uri = path.path_to_uri(encoded_path) -song1_track = Track(name='song1', uri=song1_uri) -song2_track = Track(name='song2', uri=song2_uri) -song3_track = Track(name='φοο', uri=song3_uri) -song4_track = Track(name='foo bar', uri=song4_uri) -encoded_track = Track(name='æøå', uri=encoded_uri) -song1_ext_track = song1_track.replace(name='Song #1') -song2_ext_track = song2_track.replace(name='Song #2', length=60000) -encoded_ext_track = encoded_track.replace(name='æøå') +from mopidy.models import Playlist, Ref, Track -# FIXME use mock instead of tempfile.NamedTemporaryFile - -class M3UToUriTest(unittest.TestCase): - - def parse(self, name): - return translator.parse_m3u(name, data_dir) - - def test_empty_file(self): - tracks = self.parse(path_to_data_dir('empty.m3u')) - self.assertEqual([], tracks) - - def test_basic_file(self): - tracks = self.parse(path_to_data_dir('one.m3u')) - self.assertEqual([song1_track], tracks) - - def test_file_with_comment(self): - tracks = self.parse(path_to_data_dir('comment.m3u')) - self.assertEqual([song1_track], tracks) - - def test_file_is_relative_to_correct_dir(self): - with tempfile.NamedTemporaryFile(delete=False) as tmp: - tmp.write('song1.mp3') - try: - tracks = self.parse(tmp.name) - self.assertEqual([song1_track], tracks) - finally: - if os.path.exists(tmp.name): - os.remove(tmp.name) - - def test_file_with_absolute_files(self): - with tempfile.NamedTemporaryFile(delete=False) as tmp: - tmp.write(song1_path) - try: - tracks = self.parse(tmp.name) - self.assertEqual([song1_track], tracks) - finally: - if os.path.exists(tmp.name): - os.remove(tmp.name) - - def test_file_with_multiple_absolute_files(self): - with tempfile.NamedTemporaryFile(delete=False) as tmp: - tmp.write(song1_path + '\n') - tmp.write('# comment \n') - tmp.write(song2_path) - try: - tracks = self.parse(tmp.name) - self.assertEqual([song1_track, song2_track], tracks) - finally: - if os.path.exists(tmp.name): - os.remove(tmp.name) - - def test_file_with_uri(self): - with tempfile.NamedTemporaryFile(delete=False) as tmp: - tmp.write(song1_uri) - tmp.write('\n') - tmp.write(song4_uri) - try: - tracks = self.parse(tmp.name) - self.assertEqual([song1_track, song4_track], tracks) - finally: - if os.path.exists(tmp.name): - os.remove(tmp.name) - - def test_encoding_is_latin1(self): - tracks = self.parse(path_to_data_dir('encoding.m3u')) - self.assertEqual([encoded_track], tracks) - - def test_open_missing_file(self): - tracks = self.parse(path_to_data_dir('non-existant.m3u')) - self.assertEqual([], tracks) - - def test_empty_ext_file(self): - tracks = self.parse(path_to_data_dir('empty-ext.m3u')) - self.assertEqual([], tracks) - - def test_basic_ext_file(self): - tracks = self.parse(path_to_data_dir('one-ext.m3u')) - self.assertEqual([song1_ext_track], tracks) - - def test_multi_ext_file(self): - tracks = self.parse(path_to_data_dir('two-ext.m3u')) - self.assertEqual([song1_ext_track, song2_ext_track], tracks) - - def test_ext_file_with_comment(self): - tracks = self.parse(path_to_data_dir('comment-ext.m3u')) - self.assertEqual([song1_ext_track], tracks) - - def test_ext_encoding_is_latin1(self): - 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) +def loads(s, basedir=b'.'): + return translator.load_items(io.StringIO(s), basedir=basedir) -class URItoM3UTest(unittest.TestCase): - pass +def dumps(items): + fp = io.StringIO() + translator.dump_items(items, fp) + return fp.getvalue() + + +def test_path_to_uri(): + from mopidy.m3u.translator import path_to_uri + + assert path_to_uri(b'test') == 'm3u:test' + assert path_to_uri(b'test.m3u') == 'm3u:test.m3u' + assert path_to_uri(b'./test.m3u') == 'm3u:test.m3u' + assert path_to_uri(b'foo/../test.m3u') == 'm3u:test.m3u' + assert path_to_uri(b'Test Playlist.m3u') == 'm3u:Test%20Playlist.m3u' + assert path_to_uri(b'test.mp3', scheme='file') == 'file:///test.mp3' + + +def test_latin1_path_to_uri(): + path = 'æøå.m3u'.encode('latin-1') + assert translator.path_to_uri(path) == 'm3u:%E6%F8%E5.m3u' + + +def test_utf8_path_to_uri(): + path = 'æøå.m3u'.encode('utf-8') + assert translator.path_to_uri(path) == 'm3u:%C3%A6%C3%B8%C3%A5.m3u' + + +def test_uri_to_path(): + from mopidy.m3u.translator import uri_to_path + + assert uri_to_path('m3u:test.m3u') == b'test.m3u' + assert uri_to_path(b'm3u:test.m3u') == b'test.m3u' + assert uri_to_path('m3u:Test%20Playlist.m3u') == b'Test Playlist.m3u' + assert uri_to_path(b'm3u:Test%20Playlist.m3u') == b'Test Playlist.m3u' + assert uri_to_path('m3u:%E6%F8%E5.m3u') == b'\xe6\xf8\xe5.m3u' + assert uri_to_path(b'm3u:%E6%F8%E5.m3u') == b'\xe6\xf8\xe5.m3u' + assert uri_to_path('file:///test.mp3') == b'/test.mp3' + assert uri_to_path(b'file:///test.mp3') == b'/test.mp3' + + +def test_name_from_path(): + from mopidy.m3u.translator import name_from_path + + assert name_from_path(b'test') == 'test' + assert name_from_path(b'test.m3u') == 'test' + assert name_from_path(b'../test.m3u') == 'test' + + +def test_path_from_name(): + from mopidy.m3u.translator import path_from_name + + assert path_from_name('test') == b'test' + assert path_from_name('test', '.m3u') == b'test.m3u' + assert path_from_name('foo/bar', sep='-') == b'foo-bar' + + +def test_path_to_ref(): + from mopidy.m3u.translator import path_to_ref + + assert path_to_ref(b'test.m3u') == Ref.playlist( + uri='m3u:test.m3u', name='test' + ) + assert path_to_ref(b'Test Playlist.m3u') == Ref.playlist( + uri='m3u:Test%20Playlist.m3u', name='Test Playlist' + ) + + +def test_load_items(): + assert loads('') == [] + + assert loads('test.mp3', basedir=b'/playlists') == [ + Ref.track(uri='file:///playlists/test.mp3', name='test') + ] + assert loads('../test.mp3', basedir=b'/playlists') == [ + Ref.track(uri='file:///test.mp3', name='test') + ] + assert loads('/test.mp3') == [ + Ref.track(uri='file:///test.mp3', name='test') + ] + assert loads('file:///test.mp3') == [ + Ref.track(uri='file:///test.mp3') + ] + assert loads('http://example.com/stream') == [ + Ref.track(uri='http://example.com/stream') + ] + + assert loads('#EXTM3U\n#EXTINF:42,Test\nfile:///test.mp3\n') == [ + Ref.track(uri='file:///test.mp3', name='Test') + ] + assert loads('#EXTM3U\n#EXTINF:-1,Test\nhttp://example.com/stream\n') == [ + Ref.track(uri='http://example.com/stream', name='Test') + ] + + +def test_dump_items(): + assert dumps([]) == '' + assert dumps([Ref.track(uri='file:///test.mp3')]) == ( + 'file:///test.mp3\n' + ) + assert dumps([Ref.track(uri='file:///test.mp3', name='test')]) == ( + '#EXTM3U\n' + '#EXTINF:-1,test\n' + 'file:///test.mp3\n' + ) + assert dumps([Track(uri='file:///test.mp3', name='test', length=42)]) == ( + '#EXTM3U\n' + '#EXTINF:-1,test\n' + 'file:///test.mp3\n' + ) + assert dumps([Track(uri='http://example.com/stream')]) == ( + 'http://example.com/stream\n' + ) + assert dumps([Track(uri='http://example.com/stream', name='Test')]) == ( + '#EXTM3U\n' + '#EXTINF:-1,Test\n' + 'http://example.com/stream\n' + ) + + +def test_playlist(): + from mopidy.m3u.translator import playlist + + assert playlist(b'test.m3u') == Playlist( + uri='m3u:test.m3u', + name='test' + ) + assert playlist(b'test.m3u', [Ref(uri='file:///test.mp3')], 1) == Playlist( + uri='m3u:test.m3u', + name='test', + tracks=[Track(uri='file:///test.mp3')], + last_modified=1000 + )