From e9625e9febc730229af95ce2d1036fd50530ca32 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sun, 27 Dec 2015 19:28:41 +0100 Subject: [PATCH 01/50] core: Fix #310: Persist mopidy state between runs. Persist following properties: mopidy.core.tracklist _tl_tracks _next_tlid get_consume() get_random() get_repeat() get_single() mopidy.core.history _history mopidy.core.playlist get_current_tl_track() get_time_position() mopidy.core.mixer get_volume() Details: - moved json export/import write_library()/load_library() from mopidy/local to mopidy/models - new core methods save_state(), load_state() - save_state(), load_state() accessible via rpc - save state to disk at stop - load state from disk at start - new config: core.restore_state ("off", "load", "play") TODO: - seek to play position does not work. Timing issue. - use extra thread to load state from disk at start? --- docs/api/core.rst | 3 ++ docs/api/models.rst | 7 +++ docs/config.rst | 10 +++++ mopidy/config/__init__.py | 1 + mopidy/config/default.conf | 1 + mopidy/core/actor.py | 89 ++++++++++++++++++++++++++++++++++++++ mopidy/core/history.py | 13 ++++++ mopidy/core/mixer.py | 13 ++++++ mopidy/core/playback.py | 20 +++++++++ mopidy/core/tracklist.py | 32 ++++++++++++++ mopidy/http/handlers.py | 4 ++ mopidy/local/json.py | 61 +++++++------------------- mopidy/models/storage.py | 61 ++++++++++++++++++++++++++ 13 files changed, 269 insertions(+), 46 deletions(-) create mode 100644 mopidy/models/storage.py diff --git a/docs/api/core.rst b/docs/api/core.rst index 5f1e406f..3aca2504 100644 --- a/docs/api/core.rst +++ b/docs/api/core.rst @@ -53,6 +53,9 @@ in core see :class:`~mopidy.core.CoreListener`. .. automethod:: get_version + .. automethod:: save_state + + .. automethod:: load_state Tracklist controller ==================== diff --git a/docs/api/models.rst b/docs/api/models.rst index 27c7647f..cd6d1cf2 100644 --- a/docs/api/models.rst +++ b/docs/api/models.rst @@ -87,6 +87,13 @@ Data model (de)serialization .. autoclass:: mopidy.models.ModelJSONEncoder +Data model import/export +---------------------------- + +.. autofunction:: mopidy.models.storage.save + +.. autofunction:: mopidy.models.storage.load + Data model field types ---------------------- diff --git a/docs/config.rst b/docs/config.rst index 292a6a09..50d58e39 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -110,7 +110,17 @@ 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/restore_state + Restore last state at start. Defaults to ``off``. + + Save state when Mopidy ends and restore state at next start. + Allowed values: + + - ``off``: restore nothing + - ``load``: restore settings, volume and play queue + - ``play``: restore settings, volume, play queue and start playback + Audio configuration ------------------- diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 042c20d9..e89f0eb9 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -21,6 +21,7 @@ _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) +_core_schema['restore_state'] = String(optional=True) _logging_schema = ConfigSchema('logging') _logging_schema['color'] = Boolean() diff --git a/mopidy/config/default.conf b/mopidy/config/default.conf index 675381d9..a501ccb9 100644 --- a/mopidy/config/default.conf +++ b/mopidy/config/default.conf @@ -3,6 +3,7 @@ cache_dir = $XDG_CACHE_DIR/mopidy config_dir = $XDG_CONFIG_DIR/mopidy data_dir = $XDG_DATA_DIR/mopidy max_tracklist_length = 10000 +restore_state = off [logging] color = true diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index e365e4b7..45a78baf 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals import collections import itertools import logging +import os import pykka @@ -17,6 +18,7 @@ from mopidy.core.playlists import PlaylistsController from mopidy.core.tracklist import TracklistController from mopidy.internal import versioning from mopidy.internal.deprecation import deprecated_property +from mopidy.models import storage logger = logging.getLogger(__name__) @@ -133,6 +135,93 @@ class Core( self.playback._stream_title = title CoreListener.send('stream_title_changed', title=title) + def on_start(self): + logger.debug("core on_start") + try: + amount = self._config['core']['restore_state'] + coverage = [] + if not amount or 'off' == amount: + pass + elif 'load' == amount: + coverage = ['tracklist', 'mode', 'volume', 'history'] + elif 'play' == amount: + coverage = ['tracklist', 'mode', 'autoplay', 'volume', + 'history'] + else: + logger.warn('Unknown value for config ' + 'core.restore_state: %s', amount) + if len(coverage): + self.load_state('persistent', coverage) + except Exception as e: + logger.warn('Unexpected error: %s', str(e)) + pykka.ThreadingActor.on_start(self) + + def on_stop(self): + logger.debug("core on_stop") + try: + amount = self._config['core']['restore_state'] + if amount and 'off' != amount: + self.save_state('persistent') + except Exception as e: + logger.warn('on_stop: Unexpected error: %s', str(e)) + pykka.ThreadingActor.on_stop(self) + + def save_state(self, name): + """ + Save current state to disk. + + :param name: a name (for later use with :meth:`load_state`) + :type name: str + """ + logger.info('Save state: "%s"', name) + if not name: + raise TypeError('missing file name') + + file_name = os.path.join( + self._config['core']['config_dir'], name) + file_name += '.state' + + data = {} + self.tracklist._state_export(data) + self.history._state_export(data) + self.playback._state_export(data) + self.mixer._state_export(data) + storage.save(file_name, data) + + def load_state(self, name, coverage): + """ + Restore state from disk. + + Load state from disk and restore it. Parameter `coverage` + limits the amount data to restore. Possible + values for `coverage` (list of one or more of): + + - 'tracklist' fill the tracklist + - 'mode' set tracklist properties (consume, random, repeat, single) + - 'autoplay' start playing ('tracklist' also required) + - 'volume' set mixer volume + - 'history' restore history + + :param name: a name (used previously with :meth:`save_state`) + :type path: str + :param coverage: amount of data to restore + :type coverage: list of string (see above) + """ + logger.info('Load state: "%s"', name) + if not name: + raise TypeError('missing file name') + + file_name = os.path.join( + self._config['core']['config_dir'], name) + file_name += '.state' + + data = storage.load(file_name) + self.history._state_import(data, coverage) + self.tracklist._state_import(data, coverage) + self.playback._state_import(data, coverage) + self.mixer._state_import(data, coverage) + logger.info('Load state done') + class Backends(list): diff --git a/mopidy/core/history.py b/mopidy/core/history.py index ae697e8e..a2d31cc9 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -57,3 +57,16 @@ class HistoryController(object): :rtype: list of (timestamp, :class:`mopidy.models.Ref`) tuples """ return copy.copy(self._history) + + def _state_export(self, data): + """Internal method for :class:`mopidy.Core`.""" + data['history'] = {} + data['history']['history'] = self._history + + def _state_import(self, data, coverage): + """Internal method for :class:`mopidy.Core`.""" + if 'history' not in data: + return + if 'history' in coverage: + if 'history' in data['history']: + self._history = data['history']['history'] diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py index 649ff270..787afa97 100644 --- a/mopidy/core/mixer.py +++ b/mopidy/core/mixer.py @@ -99,3 +99,16 @@ class MixerController(object): return result return False + + def _state_export(self, data): + """Internal method for :class:`mopidy.Core`.""" + data['mixer'] = {} + data['mixer']['volume'] = self.get_volume() + + def _state_import(self, data, coverage): + """Internal method for :class:`mopidy.Core`.""" + if 'mixer' not in data: + return + if 'volume' in coverage: + if 'volume' in data['mixer']: + self.set_volume(data['mixer']['volume']) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index fc20d412..b2e23fbc 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -523,3 +523,23 @@ class PlaybackController(object): def _trigger_seeked(self, time_position): logger.debug('Triggering seeked event') listener.CoreListener.send('seeked', time_position=time_position) + + def _state_export(self, data): + """Internal method for :class:`mopidy.Core`.""" + data['playback'] = {} + data['playback']['current_tl_track'] = self.get_current_tl_track() + data['playback']['position'] = self.get_time_position() + # TODO: export/import get_state()? + + def _state_import(self, data, coverage): + """Internal method for :class:`mopidy.Core`.""" + if 'playback' not in data: + return + if 'autoplay' in coverage: + if 'current_tl_track' in data['playback']: + tl_track = data['playback']['current_tl_track'] + if tl_track is not None: + self.play(tl_track=tl_track) + # TODO: Seek not working. It seeks to early. + # if 'position' in data['playback']: + # self.seek(data['playback']['position']) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 02508c97..fcd2c9e4 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -644,3 +644,35 @@ class TracklistController(object): def _trigger_options_changed(self): logger.debug('Triggering options changed event') listener.CoreListener.send('options_changed') + + def _state_export(self, data): + """Internal method for :class:`mopidy.Core`.""" + data['tracklist'] = {} + data['tracklist']['tl_tracks'] = self._tl_tracks + data['tracklist']['next_tlid'] = self._next_tlid + data['tracklist']['consume'] = self.get_consume() + data['tracklist']['random'] = self.get_random() + data['tracklist']['repeat'] = self.get_repeat() + data['tracklist']['single'] = self.get_single() + + def _state_import(self, data, coverage): + """Internal method for :class:`mopidy.Core`.""" + if 'tracklist' not in data: + return + if 'mode' in coverage: + # TODO: only one _trigger_options_changed() for all options + if 'consume' in data['tracklist']: + self.set_consume(data['tracklist']['consume']) + if 'random' in data['tracklist']: + self.set_random(data['tracklist']['random']) + if 'repeat' in data['tracklist']: + self.set_repeat(data['tracklist']['repeat']) + if 'single' in data['tracklist']: + self.set_single(data['tracklist']['single']) + if 'tracklist' in coverage: + if 'next_tlid' in data['tracklist']: + if data['tracklist']['next_tlid'] > self._next_tlid: + self._next_tlid = data['tracklist']['next_tlid'] + if 'tl_tracks' in data['tracklist']: + self._tl_tracks = data['tracklist']['tl_tracks'] + self._trigger_tracklist_changed() diff --git a/mopidy/http/handlers.py b/mopidy/http/handlers.py index a752a4f0..2994f8ed 100644 --- a/mopidy/http/handlers.py +++ b/mopidy/http/handlers.py @@ -43,6 +43,8 @@ def make_jsonrpc_wrapper(core_actor): objects={ 'core.get_uri_schemes': core.Core.get_uri_schemes, 'core.get_version': core.Core.get_version, + 'core.load_state': core.Core.load_state, + 'core.save_state': core.Core.save_state, 'core.history': core.HistoryController, 'core.library': core.LibraryController, 'core.mixer': core.MixerController, @@ -55,6 +57,8 @@ def make_jsonrpc_wrapper(core_actor): 'core.describe': inspector.describe, 'core.get_uri_schemes': core_actor.get_uri_schemes, 'core.get_version': core_actor.get_version, + 'core.load_state': core_actor.load_state, + 'core.save_state': core_actor.save_state, 'core.history': core_actor.history, 'core.library': core_actor.library, 'core.mixer': core_actor.mixer, diff --git a/mopidy/local/json.py b/mopidy/local/json.py index 8e8b5b1e..96c96e49 100644 --- a/mopidy/local/json.py +++ b/mopidy/local/json.py @@ -1,60 +1,19 @@ from __future__ import absolute_import, absolute_import, unicode_literals import collections -import gzip -import json import logging import os import re import sys -import tempfile -import mopidy from mopidy import compat, local, models -from mopidy.internal import encoding, timer +from mopidy.internal import timer from mopidy.local import search, storage, translator + logger = logging.getLogger(__name__) -# TODO: move to load and dump in models? -def load_library(json_file): - if not os.path.isfile(json_file): - logger.info( - 'No local library metadata cache found at %s. Please run ' - '`mopidy local scan` to index your local music library. ' - 'If you do not have a local music collection, you can disable the ' - 'local backend to hide this message.', - json_file) - return {} - try: - with gzip.open(json_file, 'rb') as fp: - return json.load(fp, object_hook=models.model_json_decoder) - except (IOError, ValueError) as error: - logger.warning( - 'Loading JSON local library failed: %s', - encoding.locale_decode(error)) - return {} - - -def write_library(json_file, data): - data['version'] = mopidy.__version__ - directory, basename = os.path.split(json_file) - - # TODO: cleanup directory/basename.* files. - tmp = tempfile.NamedTemporaryFile( - prefix=basename + '.', dir=directory, delete=False) - - try: - with gzip.GzipFile(fileobj=tmp, mode='wb') as fp: - json.dump(data, fp, cls=models.ModelJSONEncoder, - indent=2, separators=(',', ': ')) - os.rename(tmp.name, json_file) - finally: - if os.path.exists(tmp.name): - os.remove(tmp.name) - - class _BrowseCache(object): encoding = sys.getfilesystemencoding() splitpath_re = re.compile(r'([^/]+)') @@ -128,8 +87,18 @@ class JsonLibrary(local.Library): def load(self): logger.debug('Loading library: %s', self._json_file) with timer.time_logger('Loading tracks'): - library = load_library(self._json_file) - self._tracks = dict((t.uri, t) for t in library.get('tracks', [])) + if not os.path.isfile(self._json_file): + logger.info( + 'No local library metadata cache found at %s. Please run ' + '`mopidy local scan` to index your local music library. ' + 'If you do not have a local music collection, you can ' + 'disable the local backend to hide this message.', + self._json_file) + self._tracks = {} + else: + library = models.storage.load(self._json_file) + self._tracks = dict((t.uri, t) for t in + library.get('tracks', [])) with timer.time_logger('Building browse cache'): self._browse_cache = _BrowseCache(sorted(self._tracks.keys())) return len(self._tracks) @@ -195,7 +164,7 @@ class JsonLibrary(local.Library): self._tracks.pop(uri, None) def close(self): - write_library(self._json_file, {'tracks': self._tracks.values()}) + models.storage.save(self._json_file, {'tracks': self._tracks.values()}) def clear(self): try: diff --git a/mopidy/models/storage.py b/mopidy/models/storage.py new file mode 100644 index 00000000..20fc490f --- /dev/null +++ b/mopidy/models/storage.py @@ -0,0 +1,61 @@ +from __future__ import absolute_import, unicode_literals + +import gzip +import json +import logging +import os +import tempfile + +import mopidy +from mopidy import models +from mopidy.internal import encoding + +logger = logging.getLogger(__name__) + + +def load(path): + """ + Deserialize data from file. + + :param path: full path to import file + :type path: str + :return: deserialized data + :rtype: dict + """ + if not os.path.isfile(path): + logger.info('File does not exist: %s.', path) + return {} + try: + with gzip.open(path, 'rb') as fp: + return json.load(fp, object_hook=models.model_json_decoder) + except (IOError, ValueError) as error: + logger.warning( + 'Loading JSON failed: %s', + encoding.locale_decode(error)) + return {} + + +def save(path, data): + """ + Serialize data to file. + + :param path: full path to export file + :type path: str + :param data: dictionary containing data to save + :type data: dict + """ + data['version'] = mopidy.__version__ + directory, basename = os.path.split(path) + + # TODO: cleanup directory/basename.* files. + tmp = tempfile.NamedTemporaryFile( + prefix=basename + '.', dir=directory, delete=False) + + try: + with gzip.GzipFile(fileobj=tmp, mode='wb') as fp: + json.dump(data, fp, cls=models.ModelJSONEncoder, + indent=2, separators=(',', ': ')) + os.rename(tmp.name, path) + finally: + if os.path.exists(tmp.name): + os.remove(tmp.name) From 44841710e010c16f78745e1e917b750645fd427f Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sun, 27 Dec 2015 21:03:00 +0100 Subject: [PATCH 02/50] Use data_dir instead of config_dir. Mopidy as service can not write to config_dir. --- mopidy/core/actor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 45a78baf..12b6883d 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -178,7 +178,7 @@ class Core( raise TypeError('missing file name') file_name = os.path.join( - self._config['core']['config_dir'], name) + self._config['core']['data_dir'], name) file_name += '.state' data = {} @@ -212,7 +212,7 @@ class Core( raise TypeError('missing file name') file_name = os.path.join( - self._config['core']['config_dir'], name) + self._config['core']['data_dir'], name) file_name += '.state' data = storage.load(file_name) From a5a9178b060cd0be4eaeb0f898f2ec996a3e2397 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 2 Jan 2016 15:28:41 +0100 Subject: [PATCH 03/50] Use model(s) to save/restore current play state --- mopidy/core/actor.py | 1 + mopidy/core/history.py | 18 ++-- mopidy/core/mixer.py | 13 ++- mopidy/core/playback.py | 20 ++--- mopidy/core/tracklist.py | 47 +++++----- mopidy/models/__init__.py | 105 ++++++++++++++++++++++- mopidy/models/fields.py | 11 +++ mopidy/models/serialize.py | 4 +- tests/models/test_fields.py | 21 +++++ tests/models/test_models.py | 166 +++++++++++++++++++++++++++++++++++- 10 files changed, 351 insertions(+), 55 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 12b6883d..35035f5b 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -180,6 +180,7 @@ class Core( file_name = os.path.join( self._config['core']['data_dir'], name) file_name += '.state' + logger.info('Save state to "%s"', file_name) data = {} self.tracklist._state_export(data) diff --git a/mopidy/core/history.py b/mopidy/core/history.py index a2d31cc9..0f6c20b7 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -60,13 +60,17 @@ class HistoryController(object): def _state_export(self, data): """Internal method for :class:`mopidy.Core`.""" - data['history'] = {} - data['history']['history'] = self._history + history_list = [] + for timestamp, track in self._history: + history_list.append(models.HistoryTrack( + timestamp=timestamp, track=track)) + data['history'] = models.HistoryState(history=history_list) def _state_import(self, data, coverage): """Internal method for :class:`mopidy.Core`.""" - if 'history' not in data: - return - if 'history' in coverage: - if 'history' in data['history']: - self._history = data['history']['history'] + if 'history' in data: + hstate = data['history'] + if 'history' in coverage: + self._history = [] + for htrack in hstate.history: + self._history.append((htrack.timestamp, htrack.track)) diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py index 787afa97..48b12758 100644 --- a/mopidy/core/mixer.py +++ b/mopidy/core/mixer.py @@ -5,6 +5,7 @@ import logging from mopidy import exceptions from mopidy.internal import validation +from mopidy.models import MixerState logger = logging.getLogger(__name__) @@ -102,13 +103,11 @@ class MixerController(object): def _state_export(self, data): """Internal method for :class:`mopidy.Core`.""" - data['mixer'] = {} - data['mixer']['volume'] = self.get_volume() + data['mixer'] = MixerState(volume=self.get_volume()) def _state_import(self, data, coverage): """Internal method for :class:`mopidy.Core`.""" - if 'mixer' not in data: - return - if 'volume' in coverage: - if 'volume' in data['mixer']: - self.set_volume(data['mixer']['volume']) + if 'mixer' in data: + ms = data['mixer'] + if 'volume' in coverage: + self.set_volume(ms.volume) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index b2e23fbc..4a95f914 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -526,20 +526,18 @@ class PlaybackController(object): def _state_export(self, data): """Internal method for :class:`mopidy.Core`.""" - data['playback'] = {} - data['playback']['current_tl_track'] = self.get_current_tl_track() - data['playback']['position'] = self.get_time_position() - # TODO: export/import get_state()? + data['playback'] = models.PlaybackState( + tl_track=self.get_current_tl_track(), + position=self.get_time_position(), + state=self.get_state()) def _state_import(self, data, coverage): """Internal method for :class:`mopidy.Core`.""" - if 'playback' not in data: - return - if 'autoplay' in coverage: - if 'current_tl_track' in data['playback']: - tl_track = data['playback']['current_tl_track'] + if 'playback' in data: + ps = data['playback'] + if 'autoplay' in coverage: + tl_track = ps.tl_track if tl_track is not None: self.play(tl_track=tl_track) # TODO: Seek not working. It seeks to early. - # if 'position' in data['playback']: - # self.seek(data['playback']['position']) + # self.seek(ps.position) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index fcd2c9e4..ecc6bcdb 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -6,7 +6,7 @@ import random from mopidy import exceptions from mopidy.core import listener from mopidy.internal import deprecation, validation -from mopidy.models import TlTrack, Track +from mopidy.models import TlTrack, Track, TracklistState logger = logging.getLogger(__name__) @@ -647,32 +647,27 @@ class TracklistController(object): def _state_export(self, data): """Internal method for :class:`mopidy.Core`.""" - data['tracklist'] = {} - data['tracklist']['tl_tracks'] = self._tl_tracks - data['tracklist']['next_tlid'] = self._next_tlid - data['tracklist']['consume'] = self.get_consume() - data['tracklist']['random'] = self.get_random() - data['tracklist']['repeat'] = self.get_repeat() - data['tracklist']['single'] = self.get_single() + data['tracklist'] = TracklistState( + tracks=self._tl_tracks, + next_tlid=self._next_tlid, + consume=self.get_consume(), + random=self.get_random(), + repeat=self.get_repeat(), + single=self.get_single()) def _state_import(self, data, coverage): """Internal method for :class:`mopidy.Core`.""" - if 'tracklist' not in data: - return - if 'mode' in coverage: - # TODO: only one _trigger_options_changed() for all options - if 'consume' in data['tracklist']: - self.set_consume(data['tracklist']['consume']) - if 'random' in data['tracklist']: - self.set_random(data['tracklist']['random']) - if 'repeat' in data['tracklist']: - self.set_repeat(data['tracklist']['repeat']) - if 'single' in data['tracklist']: - self.set_single(data['tracklist']['single']) - if 'tracklist' in coverage: - if 'next_tlid' in data['tracklist']: - if data['tracklist']['next_tlid'] > self._next_tlid: - self._next_tlid = data['tracklist']['next_tlid'] - if 'tl_tracks' in data['tracklist']: - self._tl_tracks = data['tracklist']['tl_tracks'] + if 'tracklist' in data: + tls = data['tracklist'] + if 'mode' in coverage: + self.set_consume(tls.consume) + self.set_random(tls.random) + self.set_repeat(tls.repeat) + self.set_single(tls.single) + if 'tracklist' in coverage: + if tls.next_tlid > self._next_tlid: + self._next_tlid = tls.next_tlid + self._tl_tracks = [] + for track in tls.tracks: + self._tl_tracks.append(track) self._trigger_tracklist_changed() diff --git a/mopidy/models/__init__.py b/mopidy/models/__init__.py index 9f93a01b..fc6e9be6 100644 --- a/mopidy/models/__init__.py +++ b/mopidy/models/__init__.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals from mopidy import compat +from mopidy.internal import validation from mopidy.models import fields from mopidy.models.immutable import ImmutableObject, ValidatedImmutableObject from mopidy.models.serialize import ModelJSONEncoder, model_json_decoder @@ -8,7 +9,8 @@ from mopidy.models.serialize import ModelJSONEncoder, model_json_decoder __all__ = [ 'ImmutableObject', 'Ref', 'Image', 'Artist', 'Album', 'track', 'TlTrack', 'Playlist', 'SearchResult', 'model_json_decoder', 'ModelJSONEncoder', - 'ValidatedImmutableObject'] + 'ValidatedImmutableObject', 'HistoryTrack', 'HistoryState', 'MixerState', + 'PlaybackState', 'TracklistState'] class Ref(ValidatedImmutableObject): @@ -360,3 +362,104 @@ class SearchResult(ValidatedImmutableObject): # The albums matching the search query. Read-only. albums = fields.Collection(type=Album, container=tuple) + + +class HistoryTrack(ValidatedImmutableObject): + """ + A history track. Wraps a :class:`Ref` and it's timestamp. + + :param timestamp: the timestamp + :type timestamp: int + :param track: the track + :type track: :class:`Ref` + """ + + # The timestamp. Read-only. + timestamp = fields.Integer() + + # The track. Read-only. + track = fields.Field(type=Ref) + + +class HistoryState(ValidatedImmutableObject): + """ + State of the history controller. + Internally used for import/export of current state. + + :param history: the track history + :type history: list of :class:`HistoryTrack` + """ + + # The tracks. Read-only. + history = fields.Collection(type=HistoryTrack, container=tuple) + + +class MixerState(ValidatedImmutableObject): + """ + State of the mixer controller. + Internally used for import/export of current state. + + :param volume: the volume + :type volume: int + """ + + # The volume. Read-only. + volume = fields.Integer(min=0, max=100) + + +class PlaybackState(ValidatedImmutableObject): + """ + State of the playback controller. + Internally used for import/export of current state. + + :param tl_track: current track + :type tl_track: :class:`TlTrack` + :param position: play position + :type position: int + :param state: playback state + :type state: :class:`TlTrack` + """ + + # The current playing track. Read-only. + tl_track = fields.Field(type=TlTrack) + + # The playback position. Read-only. + position = fields.Integer(min=0) + + # The playback state. Read-only. + state = fields.Field(choices=validation.PLAYBACK_STATES) + + +class TracklistState(ValidatedImmutableObject): + + """ + State of the tracklist controller. + Internally used for import/export of current state. + + :param repeat: the repeat mode + :type repeat: bool + :param consume: the consume mode + :type consume: bool + :param random: the random mode + :type random: bool + :param single: the single mode + :type single: bool + """ + + # The repeat mode. Read-only. + repeat = fields.Boolean() + + # The consume mode. Read-only. + consume = fields.Boolean() + + # The random mode. Read-only. + random = fields.Boolean() + + # The single mode. Read-only. + single = fields.Boolean() + + # The repeat mode. Read-only. + next_tlid = fields.Integer(min=0) + + # The list of tracks. Read-only. + tracks = fields.Collection(type=TlTrack, container=tuple) diff --git a/mopidy/models/fields.py b/mopidy/models/fields.py index c686b447..178618d1 100644 --- a/mopidy/models/fields.py +++ b/mopidy/models/fields.py @@ -135,6 +135,17 @@ class Integer(Field): return value +class Boolean(Field): + """ + :class:`Field` for storing boolean values + + :param default: default value for field + """ + + def __init__(self, default=None): + super(Boolean, self).__init__(type=bool, default=default) + + class Collection(Field): """ :class:`Field` for storing collections of a given type. diff --git a/mopidy/models/serialize.py b/mopidy/models/serialize.py index 5002a8f7..08162db4 100644 --- a/mopidy/models/serialize.py +++ b/mopidy/models/serialize.py @@ -4,7 +4,9 @@ import json from mopidy.models import immutable -_MODELS = ['Ref', 'Artist', 'Album', 'Track', 'TlTrack', 'Playlist'] +_MODELS = ['Ref', 'Artist', 'Album', 'Track', 'TlTrack', 'Playlist', + 'HistoryTrack', 'HistoryState', 'MixerState', 'PlaybackState', + 'TracklistState'] class ModelJSONEncoder(json.JSONEncoder): diff --git a/tests/models/test_fields.py b/tests/models/test_fields.py index bf842fd5..825f66c6 100644 --- a/tests/models/test_fields.py +++ b/tests/models/test_fields.py @@ -173,6 +173,27 @@ class IntegerTest(unittest.TestCase): instance.attr = 11 +class BooleanTest(unittest.TestCase): + def test_default_handling(self): + instance = create_instance(Boolean(default=True)) + self.assertEqual(True, instance.attr) + + def test_true_allowed(self): + instance = create_instance(Boolean()) + instance.attr = True + self.assertEqual(True, instance.attr) + + def test_false_allowed(self): + instance = create_instance(Boolean()) + instance.attr = False + self.assertEqual(False, instance.attr) + + def test_int_forbidden(self): + instance = create_instance(Boolean()) + with self.assertRaises(TypeError): + instance.attr = 1 + + class CollectionTest(unittest.TestCase): def test_container_instance_is_default(self): instance = create_instance(Collection(type=int, container=frozenset)) diff --git a/tests/models/test_models.py b/tests/models/test_models.py index 5108411a..1bf2b1fb 100644 --- a/tests/models/test_models.py +++ b/tests/models/test_models.py @@ -4,8 +4,9 @@ import json import unittest from mopidy.models import ( - Album, Artist, Image, ModelJSONEncoder, Playlist, Ref, SearchResult, - TlTrack, Track, model_json_decoder) + Album, Artist, HistoryState, HistoryTrack, Image, MixerState, + ModelJSONEncoder, PlaybackState, Playlist, + Ref, SearchResult, TlTrack, Track, TracklistState, model_json_decoder) class InheritanceTest(unittest.TestCase): @@ -1168,3 +1169,164 @@ class SearchResultTest(unittest.TestCase): self.assertDictEqual( {'__model__': 'SearchResult', 'uri': 'uri'}, SearchResult(uri='uri').serialize()) + + +class HistoryTrackTest(unittest.TestCase): + + def test_track(self): + track = Ref.track() + result = HistoryTrack(track=track) + self.assertEqual(result.track, track) + with self.assertRaises(AttributeError): + result.track = None + + def test_timestamp(self): + timestamp = 1234 + result = HistoryTrack(timestamp=timestamp) + self.assertEqual(result.timestamp, timestamp) + with self.assertRaises(AttributeError): + result.timestamp = None + + +class HistoryStateTest(unittest.TestCase): + + def test_history_list(self): + history = (HistoryTrack(), + HistoryTrack()) + result = HistoryState(history=history) + self.assertEqual(result.history, history) + with self.assertRaises(AttributeError): + result.history = None + + def test_history_string_fail(self): + history = 'not_a_valid_history' + with self.assertRaises(TypeError): + HistoryState(history=history) + + +class MixerStateTest(unittest.TestCase): + + def test_volume(self): + volume = 37 + result = MixerState(volume=volume) + self.assertEqual(result.volume, volume) + with self.assertRaises(AttributeError): + result.volume = None + + def test_volume_invalid(self): + volume = 105 + with self.assertRaises(ValueError): + MixerState(volume=volume) + + +class PlaybackStateTest(unittest.TestCase): + + def test_position(self): + position = 123456 + result = PlaybackState(position=position) + self.assertEqual(result.position, position) + with self.assertRaises(AttributeError): + result.position = None + + def test_position_invalid(self): + position = -1 + with self.assertRaises(ValueError): + PlaybackState(position=position) + + def test_tl_track(self): + tl_track = TlTrack() + result = PlaybackState(tl_track=tl_track) + self.assertEqual(result.tl_track, tl_track) + with self.assertRaises(AttributeError): + result.tl_track = None + + def test_tl_track_none(self): + tl_track = None + result = PlaybackState(tl_track=tl_track) + self.assertEqual(result.tl_track, tl_track) + with self.assertRaises(AttributeError): + result.tl_track = None + + def test_tl_track_invalid(self): + tl_track = Track() + with self.assertRaises(TypeError): + PlaybackState(tl_track=tl_track) + + def test_state(self): + state = 'playing' + result = PlaybackState(state=state) + self.assertEqual(result.state, state) + with self.assertRaises(AttributeError): + result.state = None + + def test_state_invalid(self): + state = 'not_a_state' + with self.assertRaises(TypeError): + PlaybackState(state=state) + + +class TracklistStateTest(unittest.TestCase): + + def test_repeat_true(self): + repeat = True + result = TracklistState(repeat=repeat) + self.assertEqual(result.repeat, repeat) + with self.assertRaises(AttributeError): + result.repeat = None + + def test_repeat_false(self): + repeat = False + result = TracklistState(repeat=repeat) + self.assertEqual(result.repeat, repeat) + with self.assertRaises(AttributeError): + result.repeat = None + + def test_repeat_invalid(self): + repeat = 33 + with self.assertRaises(TypeError): + TracklistState(repeat=repeat) + + def test_consume_true(self): + val = True + result = TracklistState(consume=val) + self.assertEqual(result.consume, val) + with self.assertRaises(AttributeError): + result.repeat = None + + def test_random_true(self): + val = True + result = TracklistState(random=val) + self.assertEqual(result.random, val) + with self.assertRaises(AttributeError): + result.random = None + + def test_single_true(self): + val = True + result = TracklistState(single=val) + self.assertEqual(result.single, val) + with self.assertRaises(AttributeError): + result.single = None + + def test_next_tlid(self): + val = 654 + result = TracklistState(next_tlid=val) + self.assertEqual(result.next_tlid, val) + with self.assertRaises(AttributeError): + result.next_tlid = None + + def test_next_tlid_invalid(self): + val = -1 + with self.assertRaises(ValueError): + TracklistState(next_tlid=val) + + def test_tracks(self): + tracks = (TlTrack(), TlTrack()) + result = TracklistState(tracks=tracks) + self.assertEqual(result.tracks, tracks) + with self.assertRaises(AttributeError): + result.tracks = None + + def test_tracks_invalid(self): + tracks = (Track(), Track()) + with self.assertRaises(TypeError): + TracklistState(tracks=tracks) From e56c39ee78358bc4a3ce404f3f7c5eecdb5bf1dc Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sun, 3 Jan 2016 18:29:35 +0100 Subject: [PATCH 04/50] Add unit tests for export/restore core state Fix issues shown by test code --- mopidy/core/actor.py | 52 +++++++++------- mopidy/core/history.py | 13 ++-- mopidy/core/mixer.py | 14 +++-- mopidy/core/playback.py | 19 +++--- mopidy/core/tracklist.py | 25 ++++---- tests/core/test_actor.py | 28 +++++++++ tests/core/test_history.py | 59 +++++++++++++++++- tests/core/test_mixer.py | 37 ++++++++++++ tests/core/test_playback.py | 58 +++++++++++++++++- tests/core/test_tracklist.py | 112 ++++++++++++++++++++++++++++++++++- 10 files changed, 357 insertions(+), 60 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 35035f5b..e017a13b 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -138,18 +138,19 @@ class Core( def on_start(self): logger.debug("core on_start") try: - amount = self._config['core']['restore_state'] coverage = [] - if not amount or 'off' == amount: - pass - elif 'load' == amount: - coverage = ['tracklist', 'mode', 'volume', 'history'] - elif 'play' == amount: - coverage = ['tracklist', 'mode', 'autoplay', 'volume', - 'history'] - else: - logger.warn('Unknown value for config ' - 'core.restore_state: %s', amount) + if self._config and 'restore_state' in self._config['core']: + amount = self._config['core']['restore_state'] + if not amount or 'off' == amount: + pass + elif 'load' == amount: + coverage = ['tracklist', 'mode', 'volume', 'history'] + elif 'play' == amount: + coverage = ['tracklist', 'mode', 'autoplay', 'volume', + 'history'] + else: + logger.warn('Unknown value for config ' + 'core.restore_state: %s', amount) if len(coverage): self.load_state('persistent', coverage) except Exception as e: @@ -159,9 +160,10 @@ class Core( def on_stop(self): logger.debug("core on_stop") try: - amount = self._config['core']['restore_state'] - if amount and 'off' != amount: - self.save_state('persistent') + if self._config and 'restore_state' in self._config['core']: + amount = self._config['core']['restore_state'] + if amount and 'off' != amount: + self.save_state('persistent') except Exception as e: logger.warn('on_stop: Unexpected error: %s', str(e)) pykka.ThreadingActor.on_stop(self) @@ -183,10 +185,10 @@ class Core( logger.info('Save state to "%s"', file_name) data = {} - self.tracklist._state_export(data) - self.history._state_export(data) - self.playback._state_export(data) - self.mixer._state_export(data) + data['tracklist'] = self.tracklist._export_state() + data['history'] = self.history._export_state() + data['playback'] = self.playback._export_state() + data['mixer'] = self.mixer._export_state() storage.save(file_name, data) def load_state(self, name, coverage): @@ -217,11 +219,15 @@ class Core( file_name += '.state' data = storage.load(file_name) - self.history._state_import(data, coverage) - self.tracklist._state_import(data, coverage) - self.playback._state_import(data, coverage) - self.mixer._state_import(data, coverage) - logger.info('Load state done') + if 'history' in data: + self.history._restore_state(data['history'], coverage) + if 'tracklist' in data: + self.tracklist._restore_state(data['tracklist'], coverage) + if 'playback' in data: + self.playback._restore_state(data['playback'], coverage) + if 'mixer' in data: + self.mixer._restore_state(data['mixer'], coverage) + logger.debug('Load state done. file_name="%s"', file_name) class Backends(list): diff --git a/mopidy/core/history.py b/mopidy/core/history.py index 0f6c20b7..7cd62131 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -58,19 +58,20 @@ class HistoryController(object): """ return copy.copy(self._history) - def _state_export(self, data): + def _export_state(self): """Internal method for :class:`mopidy.Core`.""" history_list = [] for timestamp, track in self._history: history_list.append(models.HistoryTrack( timestamp=timestamp, track=track)) - data['history'] = models.HistoryState(history=history_list) + return models.HistoryState(history=history_list) - def _state_import(self, data, coverage): + def _restore_state(self, state, coverage): """Internal method for :class:`mopidy.Core`.""" - if 'history' in data: - hstate = data['history'] + if state: + if not isinstance(state, models.HistoryState): + raise TypeError('Expect an argument of type "HistoryState"') if 'history' in coverage: self._history = [] - for htrack in hstate.history: + for htrack in state.history: self._history.append((htrack.timestamp, htrack.track)) diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py index 48b12758..92938bf1 100644 --- a/mopidy/core/mixer.py +++ b/mopidy/core/mixer.py @@ -101,13 +101,15 @@ class MixerController(object): return False - def _state_export(self, data): + def _export_state(self): """Internal method for :class:`mopidy.Core`.""" - data['mixer'] = MixerState(volume=self.get_volume()) + return MixerState(volume=self.get_volume()) - def _state_import(self, data, coverage): + def _restore_state(self, state, coverage): """Internal method for :class:`mopidy.Core`.""" - if 'mixer' in data: - ms = data['mixer'] + if state: + if not isinstance(state, MixerState): + raise TypeError('Expect an argument of type "MixerState"') if 'volume' in coverage: - self.set_volume(ms.volume) + if state.volume: + self.set_volume(state.volume) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 4a95f914..33f37802 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -524,20 +524,19 @@ class PlaybackController(object): logger.debug('Triggering seeked event') listener.CoreListener.send('seeked', time_position=time_position) - def _state_export(self, data): + def _export_state(self): """Internal method for :class:`mopidy.Core`.""" - data['playback'] = models.PlaybackState( + return models.PlaybackState( tl_track=self.get_current_tl_track(), position=self.get_time_position(), state=self.get_state()) - def _state_import(self, data, coverage): + def _restore_state(self, state, coverage): """Internal method for :class:`mopidy.Core`.""" - if 'playback' in data: - ps = data['playback'] + if state: + if not isinstance(state, models.PlaybackState): + raise TypeError('Expect an argument of type "PlaybackState"') if 'autoplay' in coverage: - tl_track = ps.tl_track - if tl_track is not None: - self.play(tl_track=tl_track) - # TODO: Seek not working. It seeks to early. - # self.seek(ps.position) + if state.tl_track is not None: + self.play(tl_track=state.tl_track) + # TODO: seek to state.position? diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index ecc6bcdb..cc3b8bef 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -645,9 +645,9 @@ class TracklistController(object): logger.debug('Triggering options changed event') listener.CoreListener.send('options_changed') - def _state_export(self, data): + def _export_state(self): """Internal method for :class:`mopidy.Core`.""" - data['tracklist'] = TracklistState( + return TracklistState( tracks=self._tl_tracks, next_tlid=self._next_tlid, consume=self.get_consume(), @@ -655,19 +655,20 @@ class TracklistController(object): repeat=self.get_repeat(), single=self.get_single()) - def _state_import(self, data, coverage): + def _restore_state(self, state, coverage): """Internal method for :class:`mopidy.Core`.""" - if 'tracklist' in data: - tls = data['tracklist'] + if state: + if not isinstance(state, TracklistState): + raise TypeError('Expect an argument of type "TracklistState"') if 'mode' in coverage: - self.set_consume(tls.consume) - self.set_random(tls.random) - self.set_repeat(tls.repeat) - self.set_single(tls.single) + self.set_consume(state.consume) + self.set_random(state.random) + self.set_repeat(state.repeat) + self.set_single(state.single) if 'tracklist' in coverage: - if tls.next_tlid > self._next_tlid: - self._next_tlid = tls.next_tlid + if state.next_tlid > self._next_tlid: + self._next_tlid = state.next_tlid self._tl_tracks = [] - for track in tls.tracks: + for track in state.tracks: self._tl_tracks.append(track) self._trigger_tracklist_changed() diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index 8f062fa2..fda24c4c 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -1,5 +1,7 @@ from __future__ import absolute_import, unicode_literals +import shutil +import tempfile import unittest import mock @@ -43,3 +45,29 @@ class CoreActorTest(unittest.TestCase): def test_version(self): self.assertEqual(self.core.version, versioning.get_version()) + + +class CoreActorExportRestoreTest(unittest.TestCase): + + def setUp(self): + self.temp_dir = tempfile.mkdtemp() + config = { + 'core': { + 'max_tracklist_length': 10000, + 'restore_state': 'play', + 'data_dir': self.temp_dir, + } + } + + self.core = Core.start( + config=config, mixer=None, backends=[]).proxy() + + def tearDown(self): # noqa: N802 + pykka.ActorRegistry.stop_all() + shutil.rmtree(self.temp_dir) + + def test_restore_on_start(self): + # cover mopidy.core.actor.on_start and .on_stop + # starting the actor by calling any function: + self.core.get_version() + pass diff --git a/tests/core/test_history.py b/tests/core/test_history.py index 7f034cad..8c204270 100644 --- a/tests/core/test_history.py +++ b/tests/core/test_history.py @@ -4,7 +4,7 @@ import unittest from mopidy import compat from mopidy.core import HistoryController -from mopidy.models import Artist, Track +from mopidy.models import Artist, HistoryState, HistoryTrack, Ref, Track class PlaybackHistoryTest(unittest.TestCase): @@ -46,3 +46,60 @@ class PlaybackHistoryTest(unittest.TestCase): self.assertIn(track.name, ref.name) for artist in track.artists: self.assertIn(artist.name, ref.name) + + +class CoreHistoryExportRestoreTest(unittest.TestCase): + + def setUp(self): # noqa: N802 + self.tracks = [ + Track(uri='dummy1:a', name='foober'), + Track(uri='dummy2:a', name='foo'), + Track(uri='dummy3:a', name='bar') + ] + + self.refs = [] + for t in self.tracks: + self.refs.append(Ref.track(uri=t.uri, name=t.name)) + + self.history = HistoryController() + + def test_export(self): + self.history._add_track(self.tracks[2]) + self.history._add_track(self.tracks[1]) + + value = self.history._export_state() + + self.assertEqual(len(value.history), 2) + # last in, first out + self.assertEqual(value.history[0].track, self.refs[1]) + self.assertEqual(value.history[1].track, self.refs[2]) + + def test_import(self): + state = HistoryState(history=[ + HistoryTrack(timestamp=34, track=self.refs[0]), + HistoryTrack(timestamp=45, track=self.refs[2]), + HistoryTrack(timestamp=56, track=self.refs[1])]) + coverage = ['history'] + self.history._restore_state(state, coverage) + + hist = self.history.get_history() + self.assertEqual(len(hist), 3) + self.assertEqual(hist[0], (34, self.refs[0])) + self.assertEqual(hist[1], (45, self.refs[2])) + self.assertEqual(hist[2], (56, self.refs[1])) + + # after import, adding more tracks must be possible + self.history._add_track(self.tracks[1]) + hist = self.history.get_history() + self.assertEqual(len(hist), 4) + self.assertEqual(hist[0][1], self.refs[1]) + self.assertEqual(hist[1], (34, self.refs[0])) + self.assertEqual(hist[2], (45, self.refs[2])) + self.assertEqual(hist[3], (56, self.refs[1])) + + def test_import_invalid_type(self): + with self.assertRaises(TypeError): + self.history._restore_state(11, None) + + def test_import_none(self): + self.history._restore_state(None, None) diff --git a/tests/core/test_mixer.py b/tests/core/test_mixer.py index 45241fec..dbfdd656 100644 --- a/tests/core/test_mixer.py +++ b/tests/core/test_mixer.py @@ -7,6 +7,7 @@ import mock import pykka from mopidy import core, mixer +from mopidy.models import MixerState from tests import dummy_mixer @@ -154,3 +155,39 @@ class SetMuteBadBackendTest(MockBackendCoreMixerBase): def test_backend_returns_wrong_type(self): self.mixer.set_mute.return_value.get.return_value = 'done' self.assertFalse(self.core.mixer.set_mute(True)) + + +class CoreMixerExportRestoreTest(unittest.TestCase): + + def setUp(self): # noqa: N802 + self.mixer = dummy_mixer.create_proxy() + self.core = core.Core(mixer=self.mixer, backends=[]) + + def test_export(self): + volume = 32 + target = MixerState(volume=volume) + self.core.mixer.set_volume(volume) + value = self.core.mixer._export_state() + self.assertEqual(target, value) + + def test_import(self): + self.core.mixer.set_volume(11) + volume = 45 + target = MixerState(volume=volume) + coverage = ['volume'] + self.core.mixer._restore_state(target, coverage) + self.assertEqual(volume, self.core.mixer.get_volume()) + + def test_import_not_covered(self): + self.core.mixer.set_volume(21) + target = MixerState(volume=56) + coverage = ['other'] + self.core.mixer._restore_state(target, coverage) + self.assertEqual(21, self.core.mixer.get_volume()) + + def test_import_invalid_type(self): + with self.assertRaises(TypeError): + self.core.mixer._restore_state(11, None) + + def test_import_none(self): + self.core.mixer._restore_state(None, None) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 0869b3ec..a9c9ce9e 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -8,7 +8,7 @@ import pykka from mopidy import backend, core from mopidy.internal import deprecation -from mopidy.models import Track +from mopidy.models import PlaybackState, Track from tests import dummy_audio @@ -874,3 +874,59 @@ class Bug1177RegressionTest(unittest.TestCase): c.playback.pause() c.playback.next() b.playback.change_track.assert_called_once_with(track2) + + +class CorePlaybackExportRestoreTest(BaseTest): + + def test_export(self): + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[1]) + self.replay_events() + + state = PlaybackState( + position=0, state='playing', tl_track=tl_tracks[1]) + value = self.core.playback._export_state() + + self.assertEqual(state, value) + + def test_import(self): + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.stop() + self.replay_events() + self.assertEqual('stopped', self.core.playback.get_state()) + + state = PlaybackState( + position=0, state='playing', tl_track=tl_tracks[2]) + coverage = ['autoplay'] + self.core.playback._restore_state(state, coverage) + self.replay_events() + + self.assertEqual('playing', self.core.playback.get_state()) + self.assertEqual(tl_tracks[2], + self.core.playback.get_current_tl_track()) + + def test_import_not_covered(self): + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.stop() + self.replay_events() + self.assertEqual('stopped', self.core.playback.get_state()) + + state = PlaybackState( + position=0, state='playing', tl_track=tl_tracks[2]) + coverage = ['other'] + self.core.playback._restore_state(state, coverage) + self.replay_events() + + self.assertEqual('stopped', self.core.playback.get_state()) + self.assertEqual(None, + self.core.playback.get_current_tl_track()) + + def test_import_invalid_type(self): + with self.assertRaises(TypeError): + self.core.playback._restore_state(11, None) + + def test_import_none(self): + self.core.playback._restore_state(None, None) diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 24edb2e7..59f78d01 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -6,7 +6,7 @@ import mock from mopidy import backend, core from mopidy.internal import deprecation -from mopidy.models import TlTrack, Track +from mopidy.models import TlTrack, Track, TracklistState class TracklistTest(unittest.TestCase): @@ -177,3 +177,113 @@ class TracklistIndexTest(unittest.TestCase): self.assertEqual(0, self.core.tracklist.index()) self.assertEqual(1, self.core.tracklist.index()) self.assertEqual(2, self.core.tracklist.index()) + + +class TracklistExportRestoreTest(unittest.TestCase): + + def setUp(self): # noqa: N802 + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + + self.tracks = [ + Track(uri='dummy1:a', name='foo'), + Track(uri='dummy1:b', name='foo'), + Track(uri='dummy1:c', name='bar'), + ] + + self.tl_tracks = [ + TlTrack(tlid=4, track=Track(uri='first', name='First')), + TlTrack(tlid=5, track=Track(uri='second', name='Second')), + TlTrack(tlid=6, track=Track(uri='third', name='Third')), + TlTrack(tlid=8, track=Track(uri='last', name='Last')) + ] + + def lookup(uris): + return {u: [t for t in self.tracks if t.uri == u] for u in uris} + + self.core = core.Core(config, mixer=None, backends=[]) + self.core.library = mock.Mock(spec=core.LibraryController) + self.core.library.lookup.side_effect = lookup + + self.core.playback = mock.Mock(spec=core.PlaybackController) + + def test_export(self): + tl_tracks = self.core.tracklist.add(uris=[ + t.uri for t in self.tracks]) + consume = True + next_tlid = len(tl_tracks) + 1 + self.core.tracklist.set_consume(consume) + target = TracklistState(consume=consume, + repeat=False, + single=False, + random=False, + next_tlid=next_tlid, + tracks=tl_tracks) + value = self.core.tracklist._export_state() + self.assertEqual(target, value) + + def test_import(self): + target = TracklistState(consume=False, + repeat=True, + single=True, + random=False, + next_tlid=12, + tracks=self.tl_tracks) + coverage = ['mode', 'tracklist'] + self.core.tracklist._restore_state(target, coverage) + self.assertEqual(False, self.core.tracklist.get_consume()) + self.assertEqual(True, self.core.tracklist.get_repeat()) + self.assertEqual(True, self.core.tracklist.get_single()) + self.assertEqual(False, self.core.tracklist.get_random()) + self.assertEqual(12, self.core.tracklist._next_tlid) + self.assertEqual(4, self.core.tracklist.get_length()) + self.assertEqual(self.tl_tracks, self.core.tracklist.get_tl_tracks()) + + # after import, adding more tracks must be possible + self.core.tracklist.add(uris=[self.tracks[1].uri]) + self.assertEqual(13, self.core.tracklist._next_tlid) + self.assertEqual(5, self.core.tracklist.get_length()) + + def test_import_mode_only(self): + target = TracklistState(consume=False, + repeat=True, + single=True, + random=False, + next_tlid=12, + tracks=self.tl_tracks) + coverage = ['mode'] + self.core.tracklist._restore_state(target, coverage) + self.assertEqual(False, self.core.tracklist.get_consume()) + self.assertEqual(True, self.core.tracklist.get_repeat()) + self.assertEqual(True, self.core.tracklist.get_single()) + self.assertEqual(False, self.core.tracklist.get_random()) + self.assertEqual(1, self.core.tracklist._next_tlid) + self.assertEqual(0, self.core.tracklist.get_length()) + self.assertEqual([], self.core.tracklist.get_tl_tracks()) + + def test_import_tracklist_only(self): + target = TracklistState(consume=False, + repeat=True, + single=True, + random=False, + next_tlid=12, + tracks=self.tl_tracks) + coverage = ['tracklist'] + self.core.tracklist._restore_state(target, coverage) + self.assertEqual(False, self.core.tracklist.get_consume()) + self.assertEqual(False, self.core.tracklist.get_repeat()) + self.assertEqual(False, self.core.tracklist.get_single()) + self.assertEqual(False, self.core.tracklist.get_random()) + self.assertEqual(12, self.core.tracklist._next_tlid) + self.assertEqual(4, self.core.tracklist.get_length()) + self.assertEqual(self.tl_tracks, self.core.tracklist.get_tl_tracks()) + + def test_import_invalid_type(self): + with self.assertRaises(TypeError): + self.core.tracklist._restore_state(11, None) + + def test_import_none(self): + self.core.tracklist._restore_state(None, None) From 6746dd019679e9f8d50318672a761e56a6ee07a7 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Tue, 5 Jan 2016 07:41:02 +0100 Subject: [PATCH 05/50] More function for config value core.restore_state - New values for core.restore_state : "volume", "last" - Update changelog - Adjust logger output --- docs/changelog.rst | 3 +++ docs/config.rst | 5 ++++- mopidy/core/actor.py | 16 +++++++++++----- mopidy/core/playback.py | 9 +++++++-- mopidy/models/__init__.py | 2 +- 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4f49b8e5..db76c501 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,9 @@ Core API - Start ``tlid`` counting at 1 instead of 0 to keep in sync with MPD's ``songid``. +- Persist state between runs. The amount of data to persist can be + controlled by config value :confval:`core/restore_state` + Local backend -------------- diff --git a/docs/config.rst b/docs/config.rst index 50d58e39..5b7a2c29 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -118,8 +118,11 @@ Core configuration Allowed values: - ``off``: restore nothing + - ``volume``: restore volume - ``load``: restore settings, volume and play queue - - ``play``: restore settings, volume, play queue and start playback + - ``last``: like ``load``, additional start playback if last state was + 'playing' + - ``play``: like ``load``, additional start playback Audio configuration ------------------- diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index e017a13b..cc229827 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -143,10 +143,15 @@ class Core( amount = self._config['core']['restore_state'] if not amount or 'off' == amount: pass + elif 'volume' == amount: + coverage = ['volume'] elif 'load' == amount: coverage = ['tracklist', 'mode', 'volume', 'history'] + elif 'last' == amount: + coverage = ['tracklist', 'mode', 'play-last', 'volume', + 'history'] elif 'play' == amount: - coverage = ['tracklist', 'mode', 'autoplay', 'volume', + coverage = ['tracklist', 'mode', 'play-always', 'volume', 'history'] else: logger.warn('Unknown value for config ' @@ -175,14 +180,13 @@ class Core( :param name: a name (for later use with :meth:`load_state`) :type name: str """ - logger.info('Save state: "%s"', name) if not name: raise TypeError('missing file name') file_name = os.path.join( self._config['core']['data_dir'], name) file_name += '.state' - logger.info('Save state to "%s"', file_name) + logger.info('Save state to %s', file_name) data = {} data['tracklist'] = self.tracklist._export_state() @@ -190,6 +194,7 @@ class Core( data['playback'] = self.playback._export_state() data['mixer'] = self.mixer._export_state() storage.save(file_name, data) + logger.debug('Save state done') def load_state(self, name, coverage): """ @@ -210,13 +215,13 @@ class Core( :param coverage: amount of data to restore :type coverage: list of string (see above) """ - logger.info('Load state: "%s"', name) if not name: raise TypeError('missing file name') file_name = os.path.join( self._config['core']['data_dir'], name) file_name += '.state' + logger.info('Load state from %s', file_name) data = storage.load(file_name) if 'history' in data: @@ -224,10 +229,11 @@ class Core( if 'tracklist' in data: self.tracklist._restore_state(data['tracklist'], coverage) if 'playback' in data: + # playback after tracklist self.playback._restore_state(data['playback'], coverage) if 'mixer' in data: self.mixer._restore_state(data['mixer'], coverage) - logger.debug('Load state done. file_name="%s"', file_name) + logger.debug('Load state done.') class Backends(list): diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 33f37802..5efbd382 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -536,7 +536,12 @@ class PlaybackController(object): if state: if not isinstance(state, models.PlaybackState): raise TypeError('Expect an argument of type "PlaybackState"') - if 'autoplay' in coverage: - if state.tl_track is not None: + new_state = '' + if 'play-always' in coverage: + new_state = PlaybackState.PLAYING + if 'play-last' in coverage: + new_state = state.state + if state.tl_track is not None: + if PlaybackState.PLAYING == new_state: self.play(tl_track=state.tl_track) # TODO: seek to state.position? diff --git a/mopidy/models/__init__.py b/mopidy/models/__init__.py index fc6e9be6..e919bbbf 100644 --- a/mopidy/models/__init__.py +++ b/mopidy/models/__init__.py @@ -417,7 +417,7 @@ class PlaybackState(ValidatedImmutableObject): :param position: play position :type position: int :param state: playback state - :type state: :class:`TlTrack` + :type state: :class:`validation.PLAYBACK_STATES` """ # The current playing track. Read-only. From d5a45516efe08f4a30e56a6bcae1ffbc48b4a603 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Tue, 5 Jan 2016 07:53:45 +0100 Subject: [PATCH 06/50] Adujst test code for testing auto-play --- tests/core/test_playback.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index a9c9ce9e..801c79f5 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -899,7 +899,7 @@ class CorePlaybackExportRestoreTest(BaseTest): state = PlaybackState( position=0, state='playing', tl_track=tl_tracks[2]) - coverage = ['autoplay'] + coverage = ['play-always'] self.core.playback._restore_state(state, coverage) self.replay_events() From 46bb780a4632967e78e329592e5c18dd20e55c43 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 9 Jan 2016 11:46:09 +0100 Subject: [PATCH 07/50] Rename TracklistState 'tracks' to 'tl_tracks' Correct documentation. --- mopidy/core/tracklist.py | 4 ++-- mopidy/models/__init__.py | 10 +++++++--- tests/core/test_tracklist.py | 8 ++++---- tests/models/test_models.py | 8 ++++---- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index cc3b8bef..50068d92 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -648,7 +648,7 @@ class TracklistController(object): def _export_state(self): """Internal method for :class:`mopidy.Core`.""" return TracklistState( - tracks=self._tl_tracks, + tl_tracks=self._tl_tracks, next_tlid=self._next_tlid, consume=self.get_consume(), random=self.get_random(), @@ -669,6 +669,6 @@ class TracklistController(object): if state.next_tlid > self._next_tlid: self._next_tlid = state.next_tlid self._tl_tracks = [] - for track in state.tracks: + for track in state.tl_tracks: self._tl_tracks.append(track) self._trigger_tracklist_changed() diff --git a/mopidy/models/__init__.py b/mopidy/models/__init__.py index 5a980866..eb396e05 100644 --- a/mopidy/models/__init__.py +++ b/mopidy/models/__init__.py @@ -374,14 +374,14 @@ class HistoryTrack(ValidatedImmutableObject): :param timestamp: the timestamp :type timestamp: int - :param track: the track + :param track: the track reference :type track: :class:`Ref` """ # The timestamp. Read-only. timestamp = fields.Integer() - # The track. Read-only. + # The track reference. Read-only. track = fields.Field(type=Ref) @@ -448,6 +448,10 @@ class TracklistState(ValidatedImmutableObject): :type random: bool :param single: the single mode :type single: bool + :param next_tlid: the single mode + :type next_tlid: bool + :param tl_tracks: the single mode + :type tl_tracks: list of :class:`TlTrack` """ # The repeat mode. Read-only. @@ -466,4 +470,4 @@ class TracklistState(ValidatedImmutableObject): next_tlid = fields.Integer(min=0) # The list of tracks. Read-only. - tracks = fields.Collection(type=TlTrack, container=tuple) + tl_tracks = fields.Collection(type=TlTrack, container=tuple) diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 59f78d01..40de840b 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -221,7 +221,7 @@ class TracklistExportRestoreTest(unittest.TestCase): single=False, random=False, next_tlid=next_tlid, - tracks=tl_tracks) + tl_tracks=tl_tracks) value = self.core.tracklist._export_state() self.assertEqual(target, value) @@ -231,7 +231,7 @@ class TracklistExportRestoreTest(unittest.TestCase): single=True, random=False, next_tlid=12, - tracks=self.tl_tracks) + tl_tracks=self.tl_tracks) coverage = ['mode', 'tracklist'] self.core.tracklist._restore_state(target, coverage) self.assertEqual(False, self.core.tracklist.get_consume()) @@ -253,7 +253,7 @@ class TracklistExportRestoreTest(unittest.TestCase): single=True, random=False, next_tlid=12, - tracks=self.tl_tracks) + tl_tracks=self.tl_tracks) coverage = ['mode'] self.core.tracklist._restore_state(target, coverage) self.assertEqual(False, self.core.tracklist.get_consume()) @@ -270,7 +270,7 @@ class TracklistExportRestoreTest(unittest.TestCase): single=True, random=False, next_tlid=12, - tracks=self.tl_tracks) + tl_tracks=self.tl_tracks) coverage = ['tracklist'] self.core.tracklist._restore_state(target, coverage) self.assertEqual(False, self.core.tracklist.get_consume()) diff --git a/tests/models/test_models.py b/tests/models/test_models.py index 1bf2b1fb..0281fd65 100644 --- a/tests/models/test_models.py +++ b/tests/models/test_models.py @@ -1321,12 +1321,12 @@ class TracklistStateTest(unittest.TestCase): def test_tracks(self): tracks = (TlTrack(), TlTrack()) - result = TracklistState(tracks=tracks) - self.assertEqual(result.tracks, tracks) + result = TracklistState(tl_tracks=tracks) + self.assertEqual(result.tl_tracks, tracks) with self.assertRaises(AttributeError): - result.tracks = None + result.tl_tracks = None def test_tracks_invalid(self): tracks = (Track(), Track()) with self.assertRaises(TypeError): - TracklistState(tracks=tracks) + TracklistState(tl_tracks=tracks) From a9327c559f1c4b796759b076d283ae6c15a7bfb7 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 9 Jan 2016 12:00:35 +0100 Subject: [PATCH 08/50] Don't use pykka callbacks on_start and on_stop. Introduce setup() and teardown() for Core. --- mopidy/commands.py | 11 ++++++++--- mopidy/core/actor.py | 30 +++++++++++++----------------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/mopidy/commands.py b/mopidy/commands.py index 4890c722..e5adbc09 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -291,6 +291,7 @@ class RootCommand(Command): mixer_class = self.get_mixer_class(config, args.registry['mixer']) backend_classes = args.registry['backend'] frontend_classes = args.registry['frontend'] + core = None exit_status_code = 0 try: @@ -316,7 +317,7 @@ class RootCommand(Command): finally: loop.quit() self.stop_frontends(frontend_classes) - self.stop_core() + self.stop_core(core) self.stop_backends(backend_classes) self.stop_audio() if mixer_class is not None: @@ -392,8 +393,10 @@ class RootCommand(Command): def start_core(self, config, mixer, backends, audio): logger.info('Starting Mopidy core') - return Core.start( + core = Core.start( config=config, mixer=mixer, backends=backends, audio=audio).proxy() + core.setup().get() + return core def start_frontends(self, config, frontend_classes, core): logger.info( @@ -410,8 +413,10 @@ class RootCommand(Command): for frontend_class in frontend_classes: process.stop_actors_by_class(frontend_class) - def stop_core(self): + def stop_core(self, core): logger.info('Stopping Mopidy core') + if core: + core.teardown().get() process.stop_actors_by_class(Core) def stop_backends(self, backend_classes): diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 817beb0f..0c877477 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -138,43 +138,39 @@ class Core( self.playback._stream_title = title CoreListener.send('stream_title_changed', title=title) - def on_start(self): - logger.debug("core on_start") + def setup(self): try: coverage = [] if self._config and 'restore_state' in self._config['core']: - amount = self._config['core']['restore_state'] - if not amount or 'off' == amount: + value = self._config['core']['restore_state'] + if not value or 'off' == value: pass - elif 'volume' == amount: + elif 'volume' == value: coverage = ['volume'] - elif 'load' == amount: + elif 'load' == value: coverage = ['tracklist', 'mode', 'volume', 'history'] - elif 'last' == amount: + elif 'last' == value: coverage = ['tracklist', 'mode', 'play-last', 'volume', 'history'] - elif 'play' == amount: + elif 'play' == value: coverage = ['tracklist', 'mode', 'play-always', 'volume', 'history'] else: logger.warn('Unknown value for config ' - 'core.restore_state: %s', amount) + 'core.restore_state: %s', value) if len(coverage): self.load_state('persistent', coverage) except Exception as e: - logger.warn('Unexpected error: %s', str(e)) - pykka.ThreadingActor.on_start(self) + logger.warn('setup: Unexpected error: %s', str(e)) - def on_stop(self): - logger.debug("core on_stop") + def teardown(self): try: if self._config and 'restore_state' in self._config['core']: amount = self._config['core']['restore_state'] if amount and 'off' != amount: self.save_state('persistent') except Exception as e: - logger.warn('on_stop: Unexpected error: %s', str(e)) - pykka.ThreadingActor.on_stop(self) + logger.warn('teardown: Unexpected error: %s', str(e)) def save_state(self, name): """ @@ -184,7 +180,7 @@ class Core( :type name: str """ if not name: - raise TypeError('missing file name') + raise TypeError('Missing file name.') file_name = os.path.join( self._config['core']['data_dir'], name) @@ -219,7 +215,7 @@ class Core( :type coverage: list of string (see above) """ if not name: - raise TypeError('missing file name') + raise TypeError('Missing file name.') file_name = os.path.join( self._config['core']['data_dir'], name) From 6e99a95aae41bb2e816ef00a977b89f93003b894 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 9 Jan 2016 12:05:14 +0100 Subject: [PATCH 09/50] Don't modify data in library function. - storage.save: Don't modify data. mopidy.__version__ has to be added by caller. - storage.load: Added a Todo. Postponed decision, if load() shall raise an exception in case of error. See PR #310. --- mopidy/core/actor.py | 3 +++ mopidy/local/json.py | 6 +++++- mopidy/models/storage.py | 3 +-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 0c877477..750b965b 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -7,6 +7,8 @@ import os import pykka +import mopidy + from mopidy import audio, backend, mixer from mopidy.audio import PlaybackState from mopidy.core.history import HistoryController @@ -188,6 +190,7 @@ class Core( logger.info('Save state to %s', file_name) data = {} + data['version'] = mopidy.__version__ data['tracklist'] = self.tracklist._export_state() data['history'] = self.history._export_state() data['playback'] = self.playback._export_state() diff --git a/mopidy/local/json.py b/mopidy/local/json.py index 96c96e49..de40e15b 100644 --- a/mopidy/local/json.py +++ b/mopidy/local/json.py @@ -6,6 +6,8 @@ import os import re import sys +import mopidy + from mopidy import compat, local, models from mopidy.internal import timer from mopidy.local import search, storage, translator @@ -164,7 +166,9 @@ class JsonLibrary(local.Library): self._tracks.pop(uri, None) def close(self): - models.storage.save(self._json_file, {'tracks': self._tracks.values()}) + models.storage.save(self._json_file, + {'version': mopidy.__version__, + 'tracks': self._tracks.values()}) def clear(self): try: diff --git a/mopidy/models/storage.py b/mopidy/models/storage.py index 20fc490f..faa53d57 100644 --- a/mopidy/models/storage.py +++ b/mopidy/models/storage.py @@ -6,7 +6,6 @@ import logging import os import tempfile -import mopidy from mopidy import models from mopidy.internal import encoding @@ -22,6 +21,7 @@ def load(path): :return: deserialized data :rtype: dict """ + # Todo: raise an exception in case of error? if not os.path.isfile(path): logger.info('File does not exist: %s.', path) return {} @@ -44,7 +44,6 @@ def save(path, data): :param data: dictionary containing data to save :type data: dict """ - data['version'] = mopidy.__version__ directory, basename = os.path.split(path) # TODO: cleanup directory/basename.* files. From abe3d67bc11e1b28d13324c0645d3c3a8a3d86d8 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 9 Jan 2016 12:07:49 +0100 Subject: [PATCH 10/50] Some smaller fixes. - Limit config core.restore_state to a known set of values. - Initialize new_state to None instead of '' --- mopidy/config/__init__.py | 3 ++- mopidy/core/playback.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index e89f0eb9..7d5b300b 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -21,7 +21,8 @@ _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) -_core_schema['restore_state'] = String(optional=True) +_core_schema['restore_state'] = String( + optional=True, choices=['off', 'volume', 'load', 'last', 'play']) _logging_schema = ConfigSchema('logging') _logging_schema['color'] = Boolean() diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 1779ed77..f06fb64c 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -554,7 +554,7 @@ class PlaybackController(object): if state: if not isinstance(state, models.PlaybackState): raise TypeError('Expect an argument of type "PlaybackState"') - new_state = '' + new_state = None if 'play-always' in coverage: new_state = PlaybackState.PLAYING if 'play-last' in coverage: From 74344f2b19d079ae0d22160e43a31e39cca20989 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 9 Jan 2016 12:52:01 +0100 Subject: [PATCH 11/50] Use tlid instead of full tl_track To export/restore the PlayState the tlid is enough. --- mopidy/core/playback.py | 6 +++--- mopidy/models/__init__.py | 8 ++++---- tests/core/test_playback.py | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index f06fb64c..3a72e2e6 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -545,7 +545,7 @@ class PlaybackController(object): def _export_state(self): """Internal method for :class:`mopidy.Core`.""" return models.PlaybackState( - tl_track=self.get_current_tl_track(), + tlid=self.get_current_tlid(), position=self.get_time_position(), state=self.get_state()) @@ -559,7 +559,7 @@ class PlaybackController(object): new_state = PlaybackState.PLAYING if 'play-last' in coverage: new_state = state.state - if state.tl_track is not None: + if state.tlid is not None: if PlaybackState.PLAYING == new_state: - self.play(tl_track=state.tl_track) + self.play(tlid=state.tlid) # TODO: seek to state.position? diff --git a/mopidy/models/__init__.py b/mopidy/models/__init__.py index eb396e05..6df3e550 100644 --- a/mopidy/models/__init__.py +++ b/mopidy/models/__init__.py @@ -416,16 +416,16 @@ class PlaybackState(ValidatedImmutableObject): State of the playback controller. Internally used for import/export of current state. - :param tl_track: current track - :type tl_track: :class:`TlTrack` + :param tlid: current track tlid + :type tlid: int :param position: play position :type position: int :param state: playback state :type state: :class:`validation.PLAYBACK_STATES` """ - # The current playing track. Read-only. - tl_track = fields.Field(type=TlTrack) + # The tlid of current playing track. Read-only. + tlid = fields.Integer(min=1) # The playback position. Read-only. position = fields.Integer(min=0) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 128ec3ce..a6af330b 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -948,7 +948,7 @@ class CorePlaybackExportRestoreTest(BaseTest): self.replay_events() state = PlaybackState( - position=0, state='playing', tl_track=tl_tracks[1]) + position=0, state='playing', tlid=tl_tracks[1].tlid) value = self.core.playback._export_state() self.assertEqual(state, value) @@ -961,7 +961,7 @@ class CorePlaybackExportRestoreTest(BaseTest): self.assertEqual('stopped', self.core.playback.get_state()) state = PlaybackState( - position=0, state='playing', tl_track=tl_tracks[2]) + position=0, state='playing', tlid=tl_tracks[2].tlid) coverage = ['play-always'] self.core.playback._restore_state(state, coverage) self.replay_events() @@ -978,7 +978,7 @@ class CorePlaybackExportRestoreTest(BaseTest): self.assertEqual('stopped', self.core.playback.get_state()) state = PlaybackState( - position=0, state='playing', tl_track=tl_tracks[2]) + position=0, state='playing', tlid=tl_tracks[2].tlid) coverage = ['other'] self.core.playback._restore_state(state, coverage) self.replay_events() From 4869619bb98abf021b34b2a8b758687adf3e2781 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sun, 10 Jan 2016 13:24:14 +0100 Subject: [PATCH 12/50] New CoreState to hold all core states - Introduce a CoreState class that holds all core states - Move xState classes to internal - Use validation.check_instance for consistent error messages - Store tlid instead of TlTrack to restore last played track --- mopidy/core/actor.py | 34 ++--- mopidy/core/history.py | 8 +- mopidy/core/mixer.py | 4 +- mopidy/core/playback.py | 6 +- mopidy/core/tracklist.py | 9 +- mopidy/internal/models.py | 142 ++++++++++++++++++ mopidy/{models => internal}/storage.py | 0 mopidy/local/json.py | 9 +- mopidy/models/__init__.py | 109 +------------- mopidy/models/serialize.py | 7 +- tests/core/test_history.py | 3 +- tests/core/test_mixer.py | 2 +- tests/core/test_playback.py | 3 +- tests/core/test_tracklist.py | 3 +- tests/internal/test_models.py | 200 +++++++++++++++++++++++++ tests/models/test_models.py | 166 +------------------- 16 files changed, 390 insertions(+), 315 deletions(-) create mode 100644 mopidy/internal/models.py rename mopidy/{models => internal}/storage.py (100%) create mode 100644 tests/internal/test_models.py diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 750b965b..8b9010ab 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -18,9 +18,9 @@ from mopidy.core.mixer import MixerController from mopidy.core.playback import PlaybackController from mopidy.core.playlists import PlaylistsController from mopidy.core.tracklist import TracklistController -from mopidy.internal import versioning +from mopidy.internal import storage, validation, versioning from mopidy.internal.deprecation import deprecated_property -from mopidy.models import storage +from mopidy.internal.models import CoreState logger = logging.getLogger(__name__) @@ -163,7 +163,7 @@ class Core( if len(coverage): self.load_state('persistent', coverage) except Exception as e: - logger.warn('setup: Unexpected error: %s', str(e)) + logger.warn('Restore state: Unexpected error: %s', str(e)) def teardown(self): try: @@ -172,7 +172,7 @@ class Core( if amount and 'off' != amount: self.save_state('persistent') except Exception as e: - logger.warn('teardown: Unexpected error: %s', str(e)) + logger.warn('Export state: Unexpected error: %s', str(e)) def save_state(self, name): """ @@ -191,12 +191,13 @@ class Core( data = {} data['version'] = mopidy.__version__ - data['tracklist'] = self.tracklist._export_state() - data['history'] = self.history._export_state() - data['playback'] = self.playback._export_state() - data['mixer'] = self.mixer._export_state() + data['state'] = CoreState( + tracklist=self.tracklist._export_state(), + history=self.history._export_state(), + playback=self.playback._export_state(), + mixer=self.mixer._export_state()) storage.save(file_name, data) - logger.debug('Save state done') + logger.debug('Save state done.') def load_state(self, name, coverage): """ @@ -226,15 +227,14 @@ class Core( logger.info('Load state from %s', file_name) data = storage.load(file_name) - if 'history' in data: - self.history._restore_state(data['history'], coverage) - if 'tracklist' in data: - self.tracklist._restore_state(data['tracklist'], coverage) - if 'playback' in data: + if 'state' in data: + core_state = data['state'] + validation.check_instance(core_state, CoreState) + self.history._restore_state(core_state.history, coverage) + self.tracklist._restore_state(core_state.tracklist, coverage) # playback after tracklist - self.playback._restore_state(data['playback'], coverage) - if 'mixer' in data: - self.mixer._restore_state(data['mixer'], coverage) + self.playback._restore_state(core_state.playback, coverage) + self.mixer._restore_state(core_state.mixer, coverage) logger.debug('Load state done.') diff --git a/mopidy/core/history.py b/mopidy/core/history.py index 7cd62131..3c0a2446 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -5,7 +5,7 @@ import logging import time from mopidy import models - +from mopidy.internal.models import HistoryState, HistoryTrack logger = logging.getLogger(__name__) @@ -62,15 +62,13 @@ class HistoryController(object): """Internal method for :class:`mopidy.Core`.""" history_list = [] for timestamp, track in self._history: - history_list.append(models.HistoryTrack( + history_list.append(HistoryTrack( timestamp=timestamp, track=track)) - return models.HistoryState(history=history_list) + return HistoryState(history=history_list) def _restore_state(self, state, coverage): """Internal method for :class:`mopidy.Core`.""" if state: - if not isinstance(state, models.HistoryState): - raise TypeError('Expect an argument of type "HistoryState"') if 'history' in coverage: self._history = [] for htrack in state.history: diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py index 92938bf1..cceb0ebe 100644 --- a/mopidy/core/mixer.py +++ b/mopidy/core/mixer.py @@ -5,7 +5,7 @@ import logging from mopidy import exceptions from mopidy.internal import validation -from mopidy.models import MixerState +from mopidy.internal.models import MixerState logger = logging.getLogger(__name__) @@ -108,8 +108,6 @@ class MixerController(object): def _restore_state(self, state, coverage): """Internal method for :class:`mopidy.Core`.""" if state: - if not isinstance(state, MixerState): - raise TypeError('Expect an argument of type "MixerState"') if 'volume' in coverage: if state.volume: self.set_volume(state.volume) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 3a72e2e6..54d30230 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -2,11 +2,10 @@ from __future__ import absolute_import, unicode_literals import logging -from mopidy import models from mopidy.audio import PlaybackState from mopidy.compat import urllib from mopidy.core import listener -from mopidy.internal import deprecation, validation +from mopidy.internal import deprecation, models, validation logger = logging.getLogger(__name__) @@ -552,14 +551,13 @@ class PlaybackController(object): def _restore_state(self, state, coverage): """Internal method for :class:`mopidy.Core`.""" if state: - if not isinstance(state, models.PlaybackState): - raise TypeError('Expect an argument of type "PlaybackState"') new_state = None if 'play-always' in coverage: new_state = PlaybackState.PLAYING if 'play-last' in coverage: new_state = state.state if state.tlid is not None: + # TODO: restore to 'paused' state if PlaybackState.PLAYING == new_state: self.play(tlid=state.tlid) # TODO: seek to state.position? diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 50068d92..f99f3917 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -6,7 +6,8 @@ import random from mopidy import exceptions from mopidy.core import listener from mopidy.internal import deprecation, validation -from mopidy.models import TlTrack, Track, TracklistState +from mopidy.internal.models import TracklistState +from mopidy.models import TlTrack, Track logger = logging.getLogger(__name__) @@ -658,8 +659,6 @@ class TracklistController(object): def _restore_state(self, state, coverage): """Internal method for :class:`mopidy.Core`.""" if state: - if not isinstance(state, TracklistState): - raise TypeError('Expect an argument of type "TracklistState"') if 'mode' in coverage: self.set_consume(state.consume) self.set_random(state.random) @@ -670,5 +669,9 @@ class TracklistController(object): self._next_tlid = state.next_tlid self._tl_tracks = [] for track in state.tl_tracks: + # TODO: check if any backend will play the track. + # Could be an issue with music streaming services + # (login), disabled extensions and automatically + # generated playlists (pandora). self._tl_tracks.append(track) self._trigger_tracklist_changed() diff --git a/mopidy/internal/models.py b/mopidy/internal/models.py new file mode 100644 index 00000000..5214cc65 --- /dev/null +++ b/mopidy/internal/models.py @@ -0,0 +1,142 @@ +from __future__ import absolute_import, unicode_literals + +from mopidy.internal import validation +from mopidy.models import Ref, TlTrack, fields +from mopidy.models.immutable import ValidatedImmutableObject + +_MODELS = ['HistoryTrack', 'HistoryState', 'MixerState', 'PlaybackState', + 'TracklistState', 'CoreState'] + + +class HistoryTrack(ValidatedImmutableObject): + """ + A history track. Wraps a :class:`Ref` and it's timestamp. + + :param timestamp: the timestamp + :type timestamp: int + :param track: the track reference + :type track: :class:`Ref` + """ + + # The timestamp. Read-only. + timestamp = fields.Integer() + + # The track reference. Read-only. + track = fields.Field(type=Ref) + + +class HistoryState(ValidatedImmutableObject): + """ + State of the history controller. + Internally used for import/export of current state. + + :param history: the track history + :type history: list of :class:`HistoryTrack` + """ + + # The tracks. Read-only. + history = fields.Collection(type=HistoryTrack, container=tuple) + + +class MixerState(ValidatedImmutableObject): + """ + State of the mixer controller. + Internally used for import/export of current state. + + :param volume: the volume + :type volume: int + """ + + # The volume. Read-only. + volume = fields.Integer(min=0, max=100) + + +class PlaybackState(ValidatedImmutableObject): + """ + State of the playback controller. + Internally used for import/export of current state. + + :param tlid: current track tlid + :type tlid: int + :param position: play position + :type position: int + :param state: playback state + :type state: :class:`validation.PLAYBACK_STATES` + """ + + # The tlid of current playing track. Read-only. + tlid = fields.Integer(min=1) + + # The playback position. Read-only. + position = fields.Integer(min=0) + + # The playback state. Read-only. + state = fields.Field(choices=validation.PLAYBACK_STATES) + + +class TracklistState(ValidatedImmutableObject): + + """ + State of the tracklist controller. + Internally used for import/export of current state. + + :param repeat: the repeat mode + :type repeat: bool + :param consume: the consume mode + :type consume: bool + :param random: the random mode + :type random: bool + :param single: the single mode + :type single: bool + :param next_tlid: the single mode + :type next_tlid: bool + :param tl_tracks: the single mode + :type tl_tracks: list of :class:`TlTrack` + """ + + # The repeat mode. Read-only. + repeat = fields.Boolean() + + # The consume mode. Read-only. + consume = fields.Boolean() + + # The random mode. Read-only. + random = fields.Boolean() + + # The single mode. Read-only. + single = fields.Boolean() + + # The repeat mode. Read-only. + next_tlid = fields.Integer(min=0) + + # The list of tracks. Read-only. + tl_tracks = fields.Collection(type=TlTrack, container=tuple) + + +class CoreState(ValidatedImmutableObject): + + """ + State of all Core controller. + Internally used for import/export of current state. + + :param history: State of the history controller + :type history: :class:`HistorState` + :param mixer: State of the mixer controller + :type mixer: :class:`MixerState` + :param playback: State of the playback controller + :type playback: :class:`PlaybackState` + :param tracklist: State of the tracklist controller + :type tracklist: :class:`TracklistState` + """ + + # State of the history controller. + history = fields.Field(type=HistoryState) + + # State of the mixer controller. + mixer = fields.Field(type=MixerState) + + # State of the playback controller. + playback = fields.Field(type=PlaybackState) + + # State of the tracklist controller. + tracklist = fields.Field(type=TracklistState) diff --git a/mopidy/models/storage.py b/mopidy/internal/storage.py similarity index 100% rename from mopidy/models/storage.py rename to mopidy/internal/storage.py diff --git a/mopidy/local/json.py b/mopidy/local/json.py index de40e15b..378cab75 100644 --- a/mopidy/local/json.py +++ b/mopidy/local/json.py @@ -9,6 +9,7 @@ import sys import mopidy from mopidy import compat, local, models +from mopidy import internal from mopidy.internal import timer from mopidy.local import search, storage, translator @@ -98,7 +99,7 @@ class JsonLibrary(local.Library): self._json_file) self._tracks = {} else: - library = models.storage.load(self._json_file) + library = internal.storage.load(self._json_file) self._tracks = dict((t.uri, t) for t in library.get('tracks', [])) with timer.time_logger('Building browse cache'): @@ -166,9 +167,9 @@ class JsonLibrary(local.Library): self._tracks.pop(uri, None) def close(self): - models.storage.save(self._json_file, - {'version': mopidy.__version__, - 'tracks': self._tracks.values()}) + internal.storage.save(self._json_file, + {'version': mopidy.__version__, + 'tracks': self._tracks.values()}) def clear(self): try: diff --git a/mopidy/models/__init__.py b/mopidy/models/__init__.py index 6df3e550..1e63d02f 100644 --- a/mopidy/models/__init__.py +++ b/mopidy/models/__init__.py @@ -1,7 +1,6 @@ from __future__ import absolute_import, unicode_literals from mopidy import compat -from mopidy.internal import validation from mopidy.models import fields from mopidy.models.immutable import ImmutableObject, ValidatedImmutableObject from mopidy.models.serialize import ModelJSONEncoder, model_json_decoder @@ -9,8 +8,7 @@ from mopidy.models.serialize import ModelJSONEncoder, model_json_decoder __all__ = [ 'ImmutableObject', 'Ref', 'Image', 'Artist', 'Album', 'track', 'TlTrack', 'Playlist', 'SearchResult', 'model_json_decoder', 'ModelJSONEncoder', - 'ValidatedImmutableObject', 'HistoryTrack', 'HistoryState', 'MixerState', - 'PlaybackState', 'TracklistState'] + 'ValidatedImmutableObject'] class Ref(ValidatedImmutableObject): @@ -366,108 +364,3 @@ class SearchResult(ValidatedImmutableObject): # The albums matching the search query. Read-only. albums = fields.Collection(type=Album, container=tuple) - - -class HistoryTrack(ValidatedImmutableObject): - """ - A history track. Wraps a :class:`Ref` and it's timestamp. - - :param timestamp: the timestamp - :type timestamp: int - :param track: the track reference - :type track: :class:`Ref` - """ - - # The timestamp. Read-only. - timestamp = fields.Integer() - - # The track reference. Read-only. - track = fields.Field(type=Ref) - - -class HistoryState(ValidatedImmutableObject): - """ - State of the history controller. - Internally used for import/export of current state. - - :param history: the track history - :type history: list of :class:`HistoryTrack` - """ - - # The tracks. Read-only. - history = fields.Collection(type=HistoryTrack, container=tuple) - - -class MixerState(ValidatedImmutableObject): - """ - State of the mixer controller. - Internally used for import/export of current state. - - :param volume: the volume - :type volume: int - """ - - # The volume. Read-only. - volume = fields.Integer(min=0, max=100) - - -class PlaybackState(ValidatedImmutableObject): - """ - State of the playback controller. - Internally used for import/export of current state. - - :param tlid: current track tlid - :type tlid: int - :param position: play position - :type position: int - :param state: playback state - :type state: :class:`validation.PLAYBACK_STATES` - """ - - # The tlid of current playing track. Read-only. - tlid = fields.Integer(min=1) - - # The playback position. Read-only. - position = fields.Integer(min=0) - - # The playback state. Read-only. - state = fields.Field(choices=validation.PLAYBACK_STATES) - - -class TracklistState(ValidatedImmutableObject): - - """ - State of the tracklist controller. - Internally used for import/export of current state. - - :param repeat: the repeat mode - :type repeat: bool - :param consume: the consume mode - :type consume: bool - :param random: the random mode - :type random: bool - :param single: the single mode - :type single: bool - :param next_tlid: the single mode - :type next_tlid: bool - :param tl_tracks: the single mode - :type tl_tracks: list of :class:`TlTrack` - """ - - # The repeat mode. Read-only. - repeat = fields.Boolean() - - # The consume mode. Read-only. - consume = fields.Boolean() - - # The random mode. Read-only. - random = fields.Boolean() - - # The single mode. Read-only. - single = fields.Boolean() - - # The repeat mode. Read-only. - next_tlid = fields.Integer(min=0) - - # The list of tracks. Read-only. - tl_tracks = fields.Collection(type=TlTrack, container=tuple) diff --git a/mopidy/models/serialize.py b/mopidy/models/serialize.py index 08162db4..68d170c4 100644 --- a/mopidy/models/serialize.py +++ b/mopidy/models/serialize.py @@ -4,9 +4,7 @@ import json from mopidy.models import immutable -_MODELS = ['Ref', 'Artist', 'Album', 'Track', 'TlTrack', 'Playlist', - 'HistoryTrack', 'HistoryState', 'MixerState', 'PlaybackState', - 'TracklistState'] +_MODELS = ['Ref', 'Artist', 'Album', 'Track', 'TlTrack', 'Playlist'] class ModelJSONEncoder(json.JSONEncoder): @@ -46,4 +44,7 @@ def model_json_decoder(dct): model_name = dct.pop('__model__') if model_name in _MODELS: return getattr(models, model_name)(**dct) + from mopidy import internal + if model_name in internal.models._MODELS: + return getattr(internal.models, model_name)(**dct) return dct diff --git a/tests/core/test_history.py b/tests/core/test_history.py index 8c204270..65babde8 100644 --- a/tests/core/test_history.py +++ b/tests/core/test_history.py @@ -4,7 +4,8 @@ import unittest from mopidy import compat from mopidy.core import HistoryController -from mopidy.models import Artist, HistoryState, HistoryTrack, Ref, Track +from mopidy.internal.models import HistoryState, HistoryTrack +from mopidy.models import Artist, Ref, Track class PlaybackHistoryTest(unittest.TestCase): diff --git a/tests/core/test_mixer.py b/tests/core/test_mixer.py index dbfdd656..0b7b789b 100644 --- a/tests/core/test_mixer.py +++ b/tests/core/test_mixer.py @@ -7,7 +7,7 @@ import mock import pykka from mopidy import core, mixer -from mopidy.models import MixerState +from mopidy.internal.models import MixerState from tests import dummy_mixer diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index a6af330b..6d66bca6 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -8,7 +8,8 @@ import pykka from mopidy import backend, core from mopidy.internal import deprecation -from mopidy.models import PlaybackState, Track +from mopidy.internal.models import PlaybackState +from mopidy.models import Track from tests import dummy_audio diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 40de840b..7b26f3d7 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -6,7 +6,8 @@ import mock from mopidy import backend, core from mopidy.internal import deprecation -from mopidy.models import TlTrack, Track, TracklistState +from mopidy.internal.models import TracklistState +from mopidy.models import TlTrack, Track class TracklistTest(unittest.TestCase): diff --git a/tests/internal/test_models.py b/tests/internal/test_models.py new file mode 100644 index 00000000..9c181bdd --- /dev/null +++ b/tests/internal/test_models.py @@ -0,0 +1,200 @@ +from __future__ import absolute_import, unicode_literals + +import json +import unittest + +from mopidy.internal.models import ( + HistoryState, HistoryTrack, MixerState, PlaybackState, TracklistState) +from mopidy.models import ( + ModelJSONEncoder, Ref, TlTrack, Track, model_json_decoder) + + +class HistoryTrackTest(unittest.TestCase): + + def test_track(self): + track = Ref.track() + result = HistoryTrack(track=track) + self.assertEqual(result.track, track) + with self.assertRaises(AttributeError): + result.track = None + + def test_timestamp(self): + timestamp = 1234 + result = HistoryTrack(timestamp=timestamp) + self.assertEqual(result.timestamp, timestamp) + with self.assertRaises(AttributeError): + result.timestamp = None + + def test_to_json_and_back(self): + result = HistoryTrack(track=Ref.track(), timestamp=1234) + serialized = json.dumps(result, cls=ModelJSONEncoder) + deserialized = json.loads(serialized, object_hook=model_json_decoder) + self.assertEqual(result, deserialized) + + +class HistoryStateTest(unittest.TestCase): + + def test_history_list(self): + history = (HistoryTrack(), + HistoryTrack()) + result = HistoryState(history=history) + self.assertEqual(result.history, history) + with self.assertRaises(AttributeError): + result.history = None + + def test_history_string_fail(self): + history = 'not_a_valid_history' + with self.assertRaises(TypeError): + HistoryState(history=history) + + def test_to_json_and_back(self): + result = HistoryState(history=(HistoryTrack(), HistoryTrack())) + serialized = json.dumps(result, cls=ModelJSONEncoder) + deserialized = json.loads(serialized, object_hook=model_json_decoder) + self.assertEqual(result, deserialized) + + +class MixerStateTest(unittest.TestCase): + + def test_volume(self): + volume = 37 + result = MixerState(volume=volume) + self.assertEqual(result.volume, volume) + with self.assertRaises(AttributeError): + result.volume = None + + def test_volume_invalid(self): + volume = 105 + with self.assertRaises(ValueError): + MixerState(volume=volume) + + def test_to_json_and_back(self): + result = MixerState(volume=77) + serialized = json.dumps(result, cls=ModelJSONEncoder) + deserialized = json.loads(serialized, object_hook=model_json_decoder) + self.assertEqual(result, deserialized) + + +class PlaybackStateTest(unittest.TestCase): + + def test_position(self): + position = 123456 + result = PlaybackState(position=position) + self.assertEqual(result.position, position) + with self.assertRaises(AttributeError): + result.position = None + + def test_position_invalid(self): + position = -1 + with self.assertRaises(ValueError): + PlaybackState(position=position) + + def test_tl_track(self): + tlid = 42 + result = PlaybackState(tlid=tlid) + self.assertEqual(result.tlid, tlid) + with self.assertRaises(AttributeError): + result.tlid = None + + def test_tl_track_none(self): + tlid = None + result = PlaybackState(tlid=tlid) + self.assertEqual(result.tlid, tlid) + with self.assertRaises(AttributeError): + result.tl_track = None + + def test_tl_track_invalid(self): + tl_track = Track() + with self.assertRaises(TypeError): + PlaybackState(tlid=tl_track) + + def test_state(self): + state = 'playing' + result = PlaybackState(state=state) + self.assertEqual(result.state, state) + with self.assertRaises(AttributeError): + result.state = None + + def test_state_invalid(self): + state = 'not_a_state' + with self.assertRaises(TypeError): + PlaybackState(state=state) + + def test_to_json_and_back(self): + result = PlaybackState(state='playing', tlid=4321) + serialized = json.dumps(result, cls=ModelJSONEncoder) + deserialized = json.loads(serialized, object_hook=model_json_decoder) + self.assertEqual(result, deserialized) + + +class TracklistStateTest(unittest.TestCase): + + def test_repeat_true(self): + repeat = True + result = TracklistState(repeat=repeat) + self.assertEqual(result.repeat, repeat) + with self.assertRaises(AttributeError): + result.repeat = None + + def test_repeat_false(self): + repeat = False + result = TracklistState(repeat=repeat) + self.assertEqual(result.repeat, repeat) + with self.assertRaises(AttributeError): + result.repeat = None + + def test_repeat_invalid(self): + repeat = 33 + with self.assertRaises(TypeError): + TracklistState(repeat=repeat) + + def test_consume_true(self): + val = True + result = TracklistState(consume=val) + self.assertEqual(result.consume, val) + with self.assertRaises(AttributeError): + result.repeat = None + + def test_random_true(self): + val = True + result = TracklistState(random=val) + self.assertEqual(result.random, val) + with self.assertRaises(AttributeError): + result.random = None + + def test_single_true(self): + val = True + result = TracklistState(single=val) + self.assertEqual(result.single, val) + with self.assertRaises(AttributeError): + result.single = None + + def test_next_tlid(self): + val = 654 + result = TracklistState(next_tlid=val) + self.assertEqual(result.next_tlid, val) + with self.assertRaises(AttributeError): + result.next_tlid = None + + def test_next_tlid_invalid(self): + val = -1 + with self.assertRaises(ValueError): + TracklistState(next_tlid=val) + + def test_tracks(self): + tracks = (TlTrack(), TlTrack()) + result = TracklistState(tl_tracks=tracks) + self.assertEqual(result.tl_tracks, tracks) + with self.assertRaises(AttributeError): + result.tl_tracks = None + + def test_tracks_invalid(self): + tracks = (Track(), Track()) + with self.assertRaises(TypeError): + TracklistState(tl_tracks=tracks) + + def test_to_json_and_back(self): + result = TracklistState(tl_tracks=(TlTrack(), TlTrack()), next_tlid=4) + serialized = json.dumps(result, cls=ModelJSONEncoder) + deserialized = json.loads(serialized, object_hook=model_json_decoder) + self.assertEqual(result, deserialized) diff --git a/tests/models/test_models.py b/tests/models/test_models.py index 0281fd65..35e77aef 100644 --- a/tests/models/test_models.py +++ b/tests/models/test_models.py @@ -4,9 +4,8 @@ import json import unittest from mopidy.models import ( - Album, Artist, HistoryState, HistoryTrack, Image, MixerState, - ModelJSONEncoder, PlaybackState, Playlist, - Ref, SearchResult, TlTrack, Track, TracklistState, model_json_decoder) + Album, Artist, Image, ModelJSONEncoder, Playlist, + Ref, SearchResult, TlTrack, Track, model_json_decoder) class InheritanceTest(unittest.TestCase): @@ -1169,164 +1168,3 @@ class SearchResultTest(unittest.TestCase): self.assertDictEqual( {'__model__': 'SearchResult', 'uri': 'uri'}, SearchResult(uri='uri').serialize()) - - -class HistoryTrackTest(unittest.TestCase): - - def test_track(self): - track = Ref.track() - result = HistoryTrack(track=track) - self.assertEqual(result.track, track) - with self.assertRaises(AttributeError): - result.track = None - - def test_timestamp(self): - timestamp = 1234 - result = HistoryTrack(timestamp=timestamp) - self.assertEqual(result.timestamp, timestamp) - with self.assertRaises(AttributeError): - result.timestamp = None - - -class HistoryStateTest(unittest.TestCase): - - def test_history_list(self): - history = (HistoryTrack(), - HistoryTrack()) - result = HistoryState(history=history) - self.assertEqual(result.history, history) - with self.assertRaises(AttributeError): - result.history = None - - def test_history_string_fail(self): - history = 'not_a_valid_history' - with self.assertRaises(TypeError): - HistoryState(history=history) - - -class MixerStateTest(unittest.TestCase): - - def test_volume(self): - volume = 37 - result = MixerState(volume=volume) - self.assertEqual(result.volume, volume) - with self.assertRaises(AttributeError): - result.volume = None - - def test_volume_invalid(self): - volume = 105 - with self.assertRaises(ValueError): - MixerState(volume=volume) - - -class PlaybackStateTest(unittest.TestCase): - - def test_position(self): - position = 123456 - result = PlaybackState(position=position) - self.assertEqual(result.position, position) - with self.assertRaises(AttributeError): - result.position = None - - def test_position_invalid(self): - position = -1 - with self.assertRaises(ValueError): - PlaybackState(position=position) - - def test_tl_track(self): - tl_track = TlTrack() - result = PlaybackState(tl_track=tl_track) - self.assertEqual(result.tl_track, tl_track) - with self.assertRaises(AttributeError): - result.tl_track = None - - def test_tl_track_none(self): - tl_track = None - result = PlaybackState(tl_track=tl_track) - self.assertEqual(result.tl_track, tl_track) - with self.assertRaises(AttributeError): - result.tl_track = None - - def test_tl_track_invalid(self): - tl_track = Track() - with self.assertRaises(TypeError): - PlaybackState(tl_track=tl_track) - - def test_state(self): - state = 'playing' - result = PlaybackState(state=state) - self.assertEqual(result.state, state) - with self.assertRaises(AttributeError): - result.state = None - - def test_state_invalid(self): - state = 'not_a_state' - with self.assertRaises(TypeError): - PlaybackState(state=state) - - -class TracklistStateTest(unittest.TestCase): - - def test_repeat_true(self): - repeat = True - result = TracklistState(repeat=repeat) - self.assertEqual(result.repeat, repeat) - with self.assertRaises(AttributeError): - result.repeat = None - - def test_repeat_false(self): - repeat = False - result = TracklistState(repeat=repeat) - self.assertEqual(result.repeat, repeat) - with self.assertRaises(AttributeError): - result.repeat = None - - def test_repeat_invalid(self): - repeat = 33 - with self.assertRaises(TypeError): - TracklistState(repeat=repeat) - - def test_consume_true(self): - val = True - result = TracklistState(consume=val) - self.assertEqual(result.consume, val) - with self.assertRaises(AttributeError): - result.repeat = None - - def test_random_true(self): - val = True - result = TracklistState(random=val) - self.assertEqual(result.random, val) - with self.assertRaises(AttributeError): - result.random = None - - def test_single_true(self): - val = True - result = TracklistState(single=val) - self.assertEqual(result.single, val) - with self.assertRaises(AttributeError): - result.single = None - - def test_next_tlid(self): - val = 654 - result = TracklistState(next_tlid=val) - self.assertEqual(result.next_tlid, val) - with self.assertRaises(AttributeError): - result.next_tlid = None - - def test_next_tlid_invalid(self): - val = -1 - with self.assertRaises(ValueError): - TracklistState(next_tlid=val) - - def test_tracks(self): - tracks = (TlTrack(), TlTrack()) - result = TracklistState(tl_tracks=tracks) - self.assertEqual(result.tl_tracks, tracks) - with self.assertRaises(AttributeError): - result.tl_tracks = None - - def test_tracks_invalid(self): - tracks = (Track(), Track()) - with self.assertRaises(TypeError): - TracklistState(tl_tracks=tracks) From 606e87b1bbbf3c97923ca24d0a523a872953c0f7 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 14 Jan 2016 19:56:38 +0100 Subject: [PATCH 13/50] Make export/restore state internal - drop filename parameter - make save_state/load_state internal - remove save_state/load_state from docu and RPC. - remove models load/save from docu - build the config path - folder for 'core' state files - move restore_state-to-coverage-translation into a method --- docs/api/core.rst | 4 --- docs/api/models.rst | 7 ---- mopidy/core/actor.py | 74 ++++++++++++++++++++-------------------- mopidy/http/handlers.py | 4 --- tests/core/test_actor.py | 36 +++++++++++++++---- 5 files changed, 67 insertions(+), 58 deletions(-) diff --git a/docs/api/core.rst b/docs/api/core.rst index de686028..abc046bd 100644 --- a/docs/api/core.rst +++ b/docs/api/core.rst @@ -53,10 +53,6 @@ in core see :class:`~mopidy.core.CoreListener`. .. automethod:: get_version - .. automethod:: save_state - - .. automethod:: load_state - Tracklist controller ==================== diff --git a/docs/api/models.rst b/docs/api/models.rst index cd6d1cf2..27c7647f 100644 --- a/docs/api/models.rst +++ b/docs/api/models.rst @@ -87,13 +87,6 @@ Data model (de)serialization .. autoclass:: mopidy.models.ModelJSONEncoder -Data model import/export ----------------------------- - -.. autofunction:: mopidy.models.storage.save - -.. autofunction:: mopidy.models.storage.load - Data model field types ---------------------- diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 8b9010ab..9c03d035 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -18,7 +18,7 @@ from mopidy.core.mixer import MixerController from mopidy.core.playback import PlaybackController from mopidy.core.playlists import PlaylistsController from mopidy.core.tracklist import TracklistController -from mopidy.internal import storage, validation, versioning +from mopidy.internal import path, storage, validation, versioning from mopidy.internal.deprecation import deprecated_property from mopidy.internal.models import CoreState @@ -144,24 +144,10 @@ class Core( try: coverage = [] if self._config and 'restore_state' in self._config['core']: - value = self._config['core']['restore_state'] - if not value or 'off' == value: - pass - elif 'volume' == value: - coverage = ['volume'] - elif 'load' == value: - coverage = ['tracklist', 'mode', 'volume', 'history'] - elif 'last' == value: - coverage = ['tracklist', 'mode', 'play-last', 'volume', - 'history'] - elif 'play' == value: - coverage = ['tracklist', 'mode', 'play-always', 'volume', - 'history'] - else: - logger.warn('Unknown value for config ' - 'core.restore_state: %s', value) + coverage = self._config_to_coverage( + self._config['core']['restore_state']) if len(coverage): - self.load_state('persistent', coverage) + self._load_state(coverage) except Exception as e: logger.warn('Restore state: Unexpected error: %s', str(e)) @@ -170,23 +156,43 @@ class Core( if self._config and 'restore_state' in self._config['core']: amount = self._config['core']['restore_state'] if amount and 'off' != amount: - self.save_state('persistent') + self._save_state() except Exception as e: - logger.warn('Export state: Unexpected error: %s', str(e)) + logger.warn('Unexpected error while saving state: %s', str(e)) - def save_state(self, name): + @staticmethod + def _config_to_coverage(value): + coverage = [] + if not value or 'off' == value: + pass + elif 'volume' == value: + coverage = ['volume'] + elif 'load' == value: + coverage = ['tracklist', 'mode', 'volume', 'history'] + elif 'last' == value: + coverage = ['tracklist', 'mode', 'play-last', 'volume', + 'history'] + elif 'play' == value: + coverage = ['tracklist', 'mode', 'play-always', 'volume', + 'history'] + else: + logger.warn('Unknown value for config ' + 'core.restore_state: %s', value) + return coverage + + def _get_data_dir(self): + # get or create data director for core + data_dir_path = bytes( + os.path.join(self._config['core']['data_dir'], 'core')) + path.get_or_create_dir(data_dir_path) + return data_dir_path + + def _save_state(self): """ Save current state to disk. - - :param name: a name (for later use with :meth:`load_state`) - :type name: str """ - if not name: - raise TypeError('Missing file name.') - file_name = os.path.join( - self._config['core']['data_dir'], name) - file_name += '.state' + file_name = os.path.join(self._get_data_dir(), 'persistent.state') logger.info('Save state to %s', file_name) data = {} @@ -199,7 +205,7 @@ class Core( storage.save(file_name, data) logger.debug('Save state done.') - def load_state(self, name, coverage): + def _load_state(self, coverage): """ Restore state from disk. @@ -213,17 +219,11 @@ class Core( - 'volume' set mixer volume - 'history' restore history - :param name: a name (used previously with :meth:`save_state`) - :type path: str :param coverage: amount of data to restore :type coverage: list of string (see above) """ - if not name: - raise TypeError('Missing file name.') - file_name = os.path.join( - self._config['core']['data_dir'], name) - file_name += '.state' + file_name = os.path.join(self._get_data_dir(), 'persistent.state') logger.info('Load state from %s', file_name) data = storage.load(file_name) diff --git a/mopidy/http/handlers.py b/mopidy/http/handlers.py index 2994f8ed..a752a4f0 100644 --- a/mopidy/http/handlers.py +++ b/mopidy/http/handlers.py @@ -43,8 +43,6 @@ def make_jsonrpc_wrapper(core_actor): objects={ 'core.get_uri_schemes': core.Core.get_uri_schemes, 'core.get_version': core.Core.get_version, - 'core.load_state': core.Core.load_state, - 'core.save_state': core.Core.save_state, 'core.history': core.HistoryController, 'core.library': core.LibraryController, 'core.mixer': core.MixerController, @@ -57,8 +55,6 @@ def make_jsonrpc_wrapper(core_actor): 'core.describe': inspector.describe, 'core.get_uri_schemes': core_actor.get_uri_schemes, 'core.get_version': core_actor.get_version, - 'core.load_state': core_actor.load_state, - 'core.save_state': core_actor.save_state, 'core.history': core_actor.history, 'core.library': core_actor.library, 'core.mixer': core_actor.mixer, diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index fda24c4c..e0e9d9a0 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -54,7 +54,7 @@ class CoreActorExportRestoreTest(unittest.TestCase): config = { 'core': { 'max_tracklist_length': 10000, - 'restore_state': 'play', + 'restore_state': 'last', 'data_dir': self.temp_dir, } } @@ -66,8 +66,32 @@ class CoreActorExportRestoreTest(unittest.TestCase): pykka.ActorRegistry.stop_all() shutil.rmtree(self.temp_dir) - def test_restore_on_start(self): - # cover mopidy.core.actor.on_start and .on_stop - # starting the actor by calling any function: - self.core.get_version() - pass + def test_export_state(self): + self.core.teardown() + # TODO: implement meaningful test + + def test_restore_state(self): + self.core.setup() + # TODO: implement meaningful test + + def test_export_coverage_none(self): + result = Core._config_to_coverage(None) + self.assertEqual(result, []) + result = Core._config_to_coverage('off') + self.assertEqual(result, []) + + def test_export_coverage(self): + result = Core._config_to_coverage('volume') + self.assertEqual(result, ['volume']) + + result = Core._config_to_coverage('load') + target = ['tracklist', 'mode', 'volume', 'history'] + self.assertEqual(result, target) + + result = Core._config_to_coverage('last') + target = ['tracklist', 'mode', 'play-last', 'volume', 'history'] + self.assertEqual(result, target) + + result = Core._config_to_coverage('play') + target = ['tracklist', 'mode', 'play-always', 'volume', 'history'] + self.assertEqual(result, target) From 49b84f4a61ed3bb2bdaa13738533904ff0ee7112 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 14 Jan 2016 22:58:41 +0100 Subject: [PATCH 14/50] Fix a flake8 error --- mopidy/audio/scan.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index ca2c308c..a54120d1 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -141,8 +141,9 @@ def _process(pipeline, timeout_ms): have_audio = False missing_message = None - types = (gst.MESSAGE_ELEMENT | gst.MESSAGE_APPLICATION | gst.MESSAGE_ERROR - | gst.MESSAGE_EOS | gst.MESSAGE_ASYNC_DONE | gst.MESSAGE_TAG) + types = (gst.MESSAGE_ELEMENT | gst.MESSAGE_APPLICATION | + gst.MESSAGE_ERROR | gst.MESSAGE_EOS | gst.MESSAGE_ASYNC_DONE | + gst.MESSAGE_TAG) previous = clock.get_time() while timeout > 0: From 3647df61c86276e5b959a536d690a7ba1ab93582 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sun, 24 Jan 2016 17:58:44 +0100 Subject: [PATCH 15/50] More stability if a backend rejects tracks - Catch exceptions raised by backend inside 'PlaybackProvider.change_track' - Avoid endless loop if 'repeat' is 'true' and not a single track is playable --- mopidy/core/playback.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index cb89658a..81eed172 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -259,7 +259,10 @@ class PlaybackController(object): backend = self._get_backend(next_tl_track) if backend: - backend.playback.change_track(next_tl_track.track).get() + try: + backend.playback.change_track(next_tl_track.track).get() + except Exception as e: + logger.error('Change track failed: %s', e) def _on_tracklist_change(self): """ @@ -343,6 +346,8 @@ class PlaybackController(object): current = self._pending_tl_track or self._current_tl_track pending = tl_track or current or self.core.tracklist.next_track(None) + # avoid endless loop if 'repeat' is 'true' and no track is playable + count = self.core.tracklist.get_length() while pending: # TODO: should we consume unplayable tracks in this loop? @@ -352,6 +357,10 @@ class PlaybackController(object): self.core.tracklist._mark_unplayable(pending) current = pending pending = self.core.tracklist.next_track(current) + count -= 1 + if not count: + logger.info('No playable track in the list.') + break # TODO return result? @@ -368,8 +377,12 @@ class PlaybackController(object): return False backend.playback.prepare_change() - if not backend.playback.change_track(pending_tl_track.track).get(): - return False # TODO: test for this path + try: + if not backend.playback.change_track(pending_tl_track.track).get(): + return False # TODO: test for this path + except Exception as e: + logger.error('Change track failed: %s', e) + return False if state == PlaybackState.PLAYING: try: From 2401229871b744d93bbc972b47813efbfc90a4fb Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 30 Jan 2016 13:13:38 +0100 Subject: [PATCH 16/50] Catch backend exceptions with a helper function --- mopidy/core/playback.py | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 81eed172..208f0619 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -1,7 +1,9 @@ from __future__ import absolute_import, unicode_literals +import contextlib import logging +from mopidy import exceptions from mopidy.audio import PlaybackState from mopidy.compat import urllib from mopidy.core import listener @@ -10,6 +12,20 @@ from mopidy.internal import deprecation, models, validation logger = logging.getLogger(__name__) +@contextlib.contextmanager +def _backend_error_handling(backend, reraise=None): + try: + yield + except exceptions.ValidationError as e: + logger.error('%s backend returned bad data: %s', + backend.actor_ref.actor_class.__name__, e) + except Exception as e: + if reraise and isinstance(e, reraise): + raise + logger.exception('%s backend caused an exception.', + backend.actor_ref.actor_class.__name__) + + class PlaybackController(object): pykka_traversable = True @@ -259,10 +275,8 @@ class PlaybackController(object): backend = self._get_backend(next_tl_track) if backend: - try: + with _backend_error_handling(backend): backend.playback.change_track(next_tl_track.track).get() - except Exception as e: - logger.error('Change track failed: %s', e) def _on_tracklist_change(self): """ @@ -377,12 +391,12 @@ class PlaybackController(object): return False backend.playback.prepare_change() - try: - if not backend.playback.change_track(pending_tl_track.track).get(): - return False # TODO: test for this path - except Exception as e: - logger.error('Change track failed: %s', e) - return False + track_change_result = False + with _backend_error_handling(backend): + track_change_result = backend.playback.change_track( + pending_tl_track.track).get() + if not track_change_result: + return False # TODO: test for this path if state == PlaybackState.PLAYING: try: From 2b3b2e5808d987a2609ab6de776402b16c97b897 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 30 Jan 2016 13:38:29 +0100 Subject: [PATCH 17/50] Add a docstring to 'setup' and 'teardown' Inform that 'setup' and 'teardown' are for internally use only. --- mopidy/core/actor.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 9c03d035..8eb3bb36 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -141,6 +141,7 @@ class Core( CoreListener.send('stream_title_changed', title=title) def setup(self): + """ Do not call this function. It is for internal use at startup.""" try: coverage = [] if self._config and 'restore_state' in self._config['core']: @@ -152,6 +153,7 @@ class Core( logger.warn('Restore state: Unexpected error: %s', str(e)) def teardown(self): + """ Do not call this function. It is for internal use at shutdown.""" try: if self._config and 'restore_state' in self._config['core']: amount = self._config['core']['restore_state'] From cfc2d59c820cecb9cfc9072dafc547ec2bedd5e4 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 30 Jan 2016 13:51:16 +0100 Subject: [PATCH 18/50] Add a TODO for missing tracks in shuffled playlist --- mopidy/core/playback.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 208f0619..240c4454 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -361,6 +361,7 @@ class PlaybackController(object): current = self._pending_tl_track or self._current_tl_track pending = tl_track or current or self.core.tracklist.next_track(None) # avoid endless loop if 'repeat' is 'true' and no track is playable + # TODO: could miss a playable track in a shuffled playlist count = self.core.tracklist.get_length() while pending: From e51b8c58be56a17f3ece5ffb91f6dfe4d05da445 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Wed, 3 Feb 2016 20:47:10 +0100 Subject: [PATCH 19/50] Fix for: mpd client starts with empty tracklist. When restoring state, increment the tracklist version number. --- 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 f99f3917..ea60c408 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -674,4 +674,4 @@ class TracklistController(object): # (login), disabled extensions and automatically # generated playlists (pandora). self._tl_tracks.append(track) - self._trigger_tracklist_changed() + self._increase_version() From 49849fa5b39664f4b66583b08c3e6f75dcb5099d Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Wed, 3 Feb 2016 20:49:43 +0100 Subject: [PATCH 20/50] Find all playable tracks in a shuffled playlist. Run two times through the tracklist to be sure to not miss a playable track. --- mopidy/core/playback.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 240c4454..2ae8dbb3 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -361,8 +361,8 @@ class PlaybackController(object): current = self._pending_tl_track or self._current_tl_track pending = tl_track or current or self.core.tracklist.next_track(None) # avoid endless loop if 'repeat' is 'true' and no track is playable - # TODO: could miss a playable track in a shuffled playlist - count = self.core.tracklist.get_length() + # * 2 -> second run to get all playable track in a shuffled playlist + count = self.core.tracklist.get_length() * 2 while pending: # TODO: should we consume unplayable tracks in this loop? From 49325c62dde3595c5057193649a87b4427110348 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 4 Feb 2016 20:54:49 +0100 Subject: [PATCH 21/50] Test tracklist.get_version() change. The tracklist version shall increase when loading state. --- tests/core/test_tracklist.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 7b26f3d7..e3beb957 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -227,6 +227,7 @@ class TracklistExportRestoreTest(unittest.TestCase): self.assertEqual(target, value) def test_import(self): + old_version = self.core.tracklist.get_version() target = TracklistState(consume=False, repeat=True, single=True, @@ -242,6 +243,7 @@ class TracklistExportRestoreTest(unittest.TestCase): self.assertEqual(12, self.core.tracklist._next_tlid) self.assertEqual(4, self.core.tracklist.get_length()) self.assertEqual(self.tl_tracks, self.core.tracklist.get_tl_tracks()) + self.assertGreater(self.core.tracklist.get_version(), old_version) # after import, adding more tracks must be possible self.core.tracklist.add(uris=[self.tracks[1].uri]) From d8405082e9d5e5f1ad157ee11e0ccf5e8ef936f5 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 4 Feb 2016 21:14:20 +0100 Subject: [PATCH 22/50] Test tracklist.get_version() change. The tracklist version shall increase when loading state. --- tests/core/test_tracklist.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index e3beb957..5e9170b2 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -251,6 +251,7 @@ class TracklistExportRestoreTest(unittest.TestCase): self.assertEqual(5, self.core.tracklist.get_length()) def test_import_mode_only(self): + old_version = self.core.tracklist.get_version() target = TracklistState(consume=False, repeat=True, single=True, @@ -266,8 +267,10 @@ class TracklistExportRestoreTest(unittest.TestCase): self.assertEqual(1, self.core.tracklist._next_tlid) self.assertEqual(0, self.core.tracklist.get_length()) self.assertEqual([], self.core.tracklist.get_tl_tracks()) + self.assertEqual(self.core.tracklist.get_version(), old_version) def test_import_tracklist_only(self): + old_version = self.core.tracklist.get_version() target = TracklistState(consume=False, repeat=True, single=True, @@ -283,6 +286,7 @@ class TracklistExportRestoreTest(unittest.TestCase): self.assertEqual(12, self.core.tracklist._next_tlid) self.assertEqual(4, self.core.tracklist.get_length()) self.assertEqual(self.tl_tracks, self.core.tracklist.get_tl_tracks()) + self.assertGreater(self.core.tracklist.get_version(), old_version) def test_import_invalid_type(self): with self.assertRaises(TypeError): From 9d8034869d3d71ba56f6599858d17680f15af763 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 6 Feb 2016 15:48:43 +0100 Subject: [PATCH 23/50] Chance type of core.restore_state config value Change to boolean to simplify the user configuration. --- docs/config.rst | 14 +++----------- mopidy/config/__init__.py | 3 +-- mopidy/config/default.conf | 2 +- mopidy/core/actor.py | 28 ++++------------------------ tests/core/test_actor.py | 24 +----------------------- 5 files changed, 10 insertions(+), 61 deletions(-) diff --git a/docs/config.rst b/docs/config.rst index 2c302acd..b7874f83 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -113,21 +113,13 @@ Core config section .. confval:: core/restore_state - Restore last state at start. Defaults to ``off``. + When set to ``true``, Mopidy saves the state when it ends and + restores the state at next start. - Save state when Mopidy ends and restore state at next start. - Allowed values: - - - ``off``: restore nothing - - ``volume``: restore volume - - ``load``: restore settings, volume and play queue - - ``last``: like ``load``, additional start playback if last state was - 'playing' - - ``play``: like ``load``, additional start playback + Default is ``false``. .. _audio-config: - Audio configuration =================== diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 7d5b300b..fac89ac9 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -21,8 +21,7 @@ _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) -_core_schema['restore_state'] = String( - optional=True, choices=['off', 'volume', 'load', 'last', 'play']) +_core_schema['restore_state'] = Boolean(optional=True) _logging_schema = ConfigSchema('logging') _logging_schema['color'] = Boolean() diff --git a/mopidy/config/default.conf b/mopidy/config/default.conf index a501ccb9..8076a0f4 100644 --- a/mopidy/config/default.conf +++ b/mopidy/config/default.conf @@ -3,7 +3,7 @@ cache_dir = $XDG_CACHE_DIR/mopidy config_dir = $XDG_CONFIG_DIR/mopidy data_dir = $XDG_DATA_DIR/mopidy max_tracklist_length = 10000 -restore_state = off +restore_state = false [logging] color = true diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 8eb3bb36..678796fb 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -145,8 +145,9 @@ class Core( try: coverage = [] if self._config and 'restore_state' in self._config['core']: - coverage = self._config_to_coverage( - self._config['core']['restore_state']) + if self._config['core']['restore_state']: + coverage = ['tracklist', 'mode', 'play-last', 'volume', + 'history'] if len(coverage): self._load_state(coverage) except Exception as e: @@ -156,32 +157,11 @@ class Core( """ Do not call this function. It is for internal use at shutdown.""" try: if self._config and 'restore_state' in self._config['core']: - amount = self._config['core']['restore_state'] - if amount and 'off' != amount: + if self._config['core']['restore_state']: self._save_state() except Exception as e: logger.warn('Unexpected error while saving state: %s', str(e)) - @staticmethod - def _config_to_coverage(value): - coverage = [] - if not value or 'off' == value: - pass - elif 'volume' == value: - coverage = ['volume'] - elif 'load' == value: - coverage = ['tracklist', 'mode', 'volume', 'history'] - elif 'last' == value: - coverage = ['tracklist', 'mode', 'play-last', 'volume', - 'history'] - elif 'play' == value: - coverage = ['tracklist', 'mode', 'play-always', 'volume', - 'history'] - else: - logger.warn('Unknown value for config ' - 'core.restore_state: %s', value) - return coverage - def _get_data_dir(self): # get or create data director for core data_dir_path = bytes( diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index e0e9d9a0..290d1c60 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -54,7 +54,7 @@ class CoreActorExportRestoreTest(unittest.TestCase): config = { 'core': { 'max_tracklist_length': 10000, - 'restore_state': 'last', + 'restore_state': True, 'data_dir': self.temp_dir, } } @@ -73,25 +73,3 @@ class CoreActorExportRestoreTest(unittest.TestCase): def test_restore_state(self): self.core.setup() # TODO: implement meaningful test - - def test_export_coverage_none(self): - result = Core._config_to_coverage(None) - self.assertEqual(result, []) - result = Core._config_to_coverage('off') - self.assertEqual(result, []) - - def test_export_coverage(self): - result = Core._config_to_coverage('volume') - self.assertEqual(result, ['volume']) - - result = Core._config_to_coverage('load') - target = ['tracklist', 'mode', 'volume', 'history'] - self.assertEqual(result, target) - - result = Core._config_to_coverage('last') - target = ['tracklist', 'mode', 'play-last', 'volume', 'history'] - self.assertEqual(result, target) - - result = Core._config_to_coverage('play') - target = ['tracklist', 'mode', 'play-always', 'volume', 'history'] - self.assertEqual(result, target) From 3bf6b9896c2fd2fd6d30cdc5aa49e8c26e3b8428 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 6 Feb 2016 16:22:54 +0100 Subject: [PATCH 24/50] Limit history to 500 track To avoid an too large history list, save/restore maximum 500 tracks. 500 tracks a 3 minuts gives 24 hours history. --- mopidy/core/history.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/mopidy/core/history.py b/mopidy/core/history.py index 3c0a2446..a6b4c817 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -60,10 +60,18 @@ class HistoryController(object): def _export_state(self): """Internal method for :class:`mopidy.Core`.""" + + # 500 tracks a 3 minutes -> 24 hours history + count_max = 500 + count = 1 history_list = [] for timestamp, track in self._history: history_list.append(HistoryTrack( timestamp=timestamp, track=track)) + count += 1 + if count_max < count: + logger.info('Limit history to %s tracks.', count_max) + break return HistoryState(history=history_list) def _restore_state(self, state, coverage): From d04ff285147d8be30d92ae45fd66d633f4d8fd57 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sun, 7 Feb 2016 17:31:38 +0100 Subject: [PATCH 25/50] Implement 'start paused' and 'start at position' --- mopidy/core/playback.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index ea6d6569..aa735262 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -45,6 +45,9 @@ class PlaybackController(object): self._last_position = None self._previous = False + self._start_at_pos = None + self._start_paused = False + if self._audio: self._audio.set_about_to_finish_callback( self._on_about_to_finish_callback) @@ -240,6 +243,13 @@ class PlaybackController(object): if self._pending_position is None: self.set_state(PlaybackState.PLAYING) self._trigger_track_playback_started() + seek_ok = False + if self._start_at_pos: + seek_ok = self.seek(self._start_at_pos) + self._start_at_pos = None + if not seek_ok and self._start_paused: + self.pause() + self._start_paused = False else: self._seek(self._pending_position) @@ -247,6 +257,9 @@ class PlaybackController(object): if self._pending_position == position: self._trigger_seeked(position) self._pending_position = None + if self._start_paused: + self._start_paused = False + self.pause() def _on_about_to_finish_callback(self): """Callback that performs a blocking actor call to the real callback. @@ -586,7 +599,9 @@ class PlaybackController(object): if 'play-last' in coverage: new_state = state.state if state.tlid is not None: - # TODO: restore to 'paused' state - if PlaybackState.PLAYING == new_state: + if PlaybackState.PAUSED == new_state: + self._start_paused = True + if (PlaybackState.PLAYING == new_state or + PlaybackState.PAUSED == new_state): + self._start_at_pos = state.position self.play(tlid=state.tlid) - # TODO: seek to state.position? From 3eac5895570e84a6ff4c7ffcebd7fcc1c6a512d9 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Mon, 29 Feb 2016 20:47:11 +0100 Subject: [PATCH 26/50] Try to restore state only one time. Delete the persistant file after read. If something goes wrong during restore, the next start is clean. --- mopidy/core/actor.py | 4 ++++ tests/core/test_actor.py | 21 ++++++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 678796fb..59f10450 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -209,6 +209,10 @@ class Core( logger.info('Load state from %s', file_name) data = storage.load(file_name) + + # Try only once. If something goes wrong, the next start is clean. + os.remove(file_name) + if 'state' in data: core_state = data['state'] validation.check_instance(core_state, CoreState) diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index 290d1c60..e935241f 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -1,5 +1,6 @@ from __future__ import absolute_import, unicode_literals +import os import shutil import tempfile import unittest @@ -9,7 +10,7 @@ import mock import pykka from mopidy.core import Core -from mopidy.internal import versioning +from mopidy.internal import storage, versioning class CoreActorTest(unittest.TestCase): @@ -59,6 +60,8 @@ class CoreActorExportRestoreTest(unittest.TestCase): } } + os.mkdir(os.path.join(self.temp_dir, 'core')) + self.core = Core.start( config=config, mixer=None, backends=[]).proxy() @@ -67,9 +70,21 @@ class CoreActorExportRestoreTest(unittest.TestCase): shutil.rmtree(self.temp_dir) def test_export_state(self): - self.core.teardown() + self.core.teardown().get() # TODO: implement meaningful test def test_restore_state(self): - self.core.setup() + self.core.setup().get() # TODO: implement meaningful test + + def test_delete_state_file_on_restore(self): + file_path = os.path.join(self.temp_dir, 'core', 'persistent.state') + + data = {} + storage.save(file_path, data) + self.assertTrue(os.path.isfile(file_path), 'missing persistent file') + + self.core.setup().get() + + self.assertFalse(os.path.isfile(file_path), + 'persistent file has to be deleted') From 58db550bd60d23025cae8ef1ece399bcd0792189 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sun, 6 Mar 2016 12:42:13 +0100 Subject: [PATCH 27/50] Register mopidy models for deserialization. All from ValidatedImmutableObject derived classes are registered for automatically deserialization by model_json_decoder(). --- mopidy/internal/models.py | 3 --- mopidy/models/immutable.py | 11 ++++++++++- mopidy/models/serialize.py | 11 +++-------- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/mopidy/internal/models.py b/mopidy/internal/models.py index 5214cc65..690b1802 100644 --- a/mopidy/internal/models.py +++ b/mopidy/internal/models.py @@ -4,9 +4,6 @@ from mopidy.internal import validation from mopidy.models import Ref, TlTrack, fields from mopidy.models.immutable import ValidatedImmutableObject -_MODELS = ['HistoryTrack', 'HistoryState', 'MixerState', 'PlaybackState', - 'TracklistState', 'CoreState'] - class HistoryTrack(ValidatedImmutableObject): """ diff --git a/mopidy/models/immutable.py b/mopidy/models/immutable.py index 18de7d76..fadff89b 100644 --- a/mopidy/models/immutable.py +++ b/mopidy/models/immutable.py @@ -8,6 +8,10 @@ from mopidy.internal import deprecation from mopidy.models.fields import Field +# Registered models for automatic deserialization +_models = {} + + class ImmutableObject(object): """ Superclass for immutable objects whose fields can only be modified via the @@ -150,9 +154,14 @@ class _ValidatedImmutableObjectMeta(type): attrs['_instances'] = weakref.WeakValueDictionary() attrs['__slots__'] = list(attrs.get('__slots__', [])) + fields.values() - return super(_ValidatedImmutableObjectMeta, cls).__new__( + clsc = super(_ValidatedImmutableObjectMeta, cls).__new__( cls, name, bases, attrs) + if clsc.__name__ != 'ValidatedImmutableObject': + _models[clsc.__name__] = clsc + + return clsc + def __call__(cls, *args, **kwargs): # noqa: N805 instance = super(_ValidatedImmutableObjectMeta, cls).__call__( *args, **kwargs) diff --git a/mopidy/models/serialize.py b/mopidy/models/serialize.py index 68d170c4..ab173aae 100644 --- a/mopidy/models/serialize.py +++ b/mopidy/models/serialize.py @@ -4,8 +4,6 @@ import json from mopidy.models import immutable -_MODELS = ['Ref', 'Artist', 'Album', 'Track', 'TlTrack', 'Playlist'] - class ModelJSONEncoder(json.JSONEncoder): @@ -40,11 +38,8 @@ def model_json_decoder(dct): """ if '__model__' in dct: - from mopidy import models model_name = dct.pop('__model__') - if model_name in _MODELS: - return getattr(models, model_name)(**dct) - from mopidy import internal - if model_name in internal.models._MODELS: - return getattr(internal.models, model_name)(**dct) + if model_name in immutable._models: + cls = immutable._models[model_name] + return cls(**dct) return dct From d4d2ad80dc4c74b8361e5ef2d789f805cc1520eb Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 31 Mar 2016 20:24:31 +0200 Subject: [PATCH 28/50] Changed name persistant file to 'state.json.gz' Changed filename from persistent.state to state.json.gz. Now the file extension matches the content. --- mopidy/core/actor.py | 4 ++-- tests/core/test_actor.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 59f10450..52c3dfb3 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -174,7 +174,7 @@ class Core( Save current state to disk. """ - file_name = os.path.join(self._get_data_dir(), 'persistent.state') + file_name = os.path.join(self._get_data_dir(), b'state.json.gz') logger.info('Save state to %s', file_name) data = {} @@ -205,7 +205,7 @@ class Core( :type coverage: list of string (see above) """ - file_name = os.path.join(self._get_data_dir(), 'persistent.state') + file_name = os.path.join(self._get_data_dir(), b'state.json.gz') logger.info('Load state from %s', file_name) data = storage.load(file_name) diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index e935241f..9d71fcef 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -78,7 +78,7 @@ class CoreActorExportRestoreTest(unittest.TestCase): # TODO: implement meaningful test def test_delete_state_file_on_restore(self): - file_path = os.path.join(self.temp_dir, 'core', 'persistent.state') + file_path = os.path.join(self.temp_dir, 'core', 'state.json.gz') data = {} storage.save(file_path, data) From b581d5a574ab90fd4cc195bc7ba438628c80ac0a Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 31 Mar 2016 20:41:47 +0200 Subject: [PATCH 29/50] Update documentation. Update persistent feature in changelog and config description. --- docs/changelog.rst | 6 ++---- docs/config.rst | 5 +++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 08f0396b..b048e726 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,7 +10,8 @@ v2.1.0 (UNRELEASED) Feature release. -- Nothing yet. +- Core: Mopidy restores its last state when started. Can be enabled by setting + the config value :confval:`core/restore_state` to `true`. v2.0.1 (UNRELEASED) @@ -95,9 +96,6 @@ 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 - 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`) diff --git a/docs/config.rst b/docs/config.rst index 08381aaa..df8e2a7d 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -113,8 +113,9 @@ Core config section .. confval:: core/restore_state - When set to ``true``, Mopidy saves the state when it ends and - restores the state at next start. + When set to ``true``, Mopidy restores its last state when started. + The restored state includes the tracklist, playback history, + the playback state, and the mixers volume and mute state. Default is ``false``. From 7928caeb7124cb0a501c4659e12e6e2a80362c2b Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 31 Mar 2016 21:30:48 +0200 Subject: [PATCH 30/50] Persist mixer mute state. Restore mixer.mute state at start. Update mopidy.models docstrings. --- mopidy/core/actor.py | 4 ++-- mopidy/core/mixer.py | 6 ++++-- mopidy/internal/models.py | 15 ++++++++++----- tests/core/test_mixer.py | 37 +++++++++++++++++++++++++++++++++---- 4 files changed, 49 insertions(+), 13 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 52c3dfb3..56fe6e74 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -146,7 +146,7 @@ class Core( coverage = [] if self._config and 'restore_state' in self._config['core']: if self._config['core']['restore_state']: - coverage = ['tracklist', 'mode', 'play-last', 'volume', + coverage = ['tracklist', 'mode', 'play-last', 'mixer', 'history'] if len(coverage): self._load_state(coverage) @@ -198,7 +198,7 @@ class Core( - 'tracklist' fill the tracklist - 'mode' set tracklist properties (consume, random, repeat, single) - 'autoplay' start playing ('tracklist' also required) - - 'volume' set mixer volume + - 'mixer' set mixer volume and mute state - 'history' restore history :param coverage: amount of data to restore diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py index cceb0ebe..3d9b7003 100644 --- a/mopidy/core/mixer.py +++ b/mopidy/core/mixer.py @@ -103,11 +103,13 @@ class MixerController(object): def _export_state(self): """Internal method for :class:`mopidy.Core`.""" - return MixerState(volume=self.get_volume()) + return MixerState(volume=self.get_volume(), + mute=self.get_mute()) def _restore_state(self, state, coverage): """Internal method for :class:`mopidy.Core`.""" if state: - if 'volume' in coverage: + if 'mixer' in coverage: if state.volume: self.set_volume(state.volume) + self.set_mute(state.mute) diff --git a/mopidy/internal/models.py b/mopidy/internal/models.py index 690b1802..b66e0504 100644 --- a/mopidy/internal/models.py +++ b/mopidy/internal/models.py @@ -7,7 +7,7 @@ from mopidy.models.immutable import ValidatedImmutableObject class HistoryTrack(ValidatedImmutableObject): """ - A history track. Wraps a :class:`Ref` and it's timestamp. + A history track. Wraps a :class:`Ref` and its timestamp. :param timestamp: the timestamp :type timestamp: int @@ -42,11 +42,16 @@ class MixerState(ValidatedImmutableObject): :param volume: the volume :type volume: int + :param mute: the volume + :type mute: int """ # The volume. Read-only. volume = fields.Integer(min=0, max=100) + # The mute state. Read-only. + mute = fields.Boolean(default=False) + class PlaybackState(ValidatedImmutableObject): """ @@ -85,9 +90,9 @@ class TracklistState(ValidatedImmutableObject): :type random: bool :param single: the single mode :type single: bool - :param next_tlid: the single mode - :type next_tlid: bool - :param tl_tracks: the single mode + :param next_tlid: the id of the next track to play + :type next_tlid: int + :param tl_tracks: the list of tracks :type tl_tracks: list of :class:`TlTrack` """ @@ -103,7 +108,7 @@ class TracklistState(ValidatedImmutableObject): # The single mode. Read-only. single = fields.Boolean() - # The repeat mode. Read-only. + # The id of the track to play. Read-only. next_tlid = fields.Integer(min=0) # The list of tracks. Read-only. diff --git a/tests/core/test_mixer.py b/tests/core/test_mixer.py index 0b7b789b..17ecdfb5 100644 --- a/tests/core/test_mixer.py +++ b/tests/core/test_mixer.py @@ -163,10 +163,21 @@ class CoreMixerExportRestoreTest(unittest.TestCase): self.mixer = dummy_mixer.create_proxy() self.core = core.Core(mixer=self.mixer, backends=[]) - def test_export(self): + def test_export_mute(self): volume = 32 - target = MixerState(volume=volume) + mute = False + target = MixerState(volume=volume, mute=mute) self.core.mixer.set_volume(volume) + self.core.mixer.set_mute(mute) + value = self.core.mixer._export_state() + self.assertEqual(target, value) + + def test_export_unmute(self): + volume = 33 + mute = True + target = MixerState(volume=volume, mute=mute) + self.core.mixer.set_volume(volume) + self.core.mixer.set_mute(mute) value = self.core.mixer._export_state() self.assertEqual(target, value) @@ -174,16 +185,34 @@ class CoreMixerExportRestoreTest(unittest.TestCase): self.core.mixer.set_volume(11) volume = 45 target = MixerState(volume=volume) - coverage = ['volume'] + coverage = ['mixer'] self.core.mixer._restore_state(target, coverage) self.assertEqual(volume, self.core.mixer.get_volume()) def test_import_not_covered(self): self.core.mixer.set_volume(21) - target = MixerState(volume=56) + self.core.mixer.set_mute(True) + target = MixerState(volume=56, mute=False) coverage = ['other'] self.core.mixer._restore_state(target, coverage) self.assertEqual(21, self.core.mixer.get_volume()) + self.assertEqual(True, self.core.mixer.get_mute()) + + def test_import_mute_on(self): + self.core.mixer.set_mute(False) + self.assertEqual(False, self.core.mixer.get_mute()) + target = MixerState(mute=True) + coverage = ['mixer'] + self.core.mixer._restore_state(target, coverage) + self.assertEqual(True, self.core.mixer.get_mute()) + + def test_import_mute_off(self): + self.core.mixer.set_mute(True) + self.assertEqual(True, self.core.mixer.get_mute()) + target = MixerState(mute=False) + coverage = ['mixer'] + self.core.mixer._restore_state(target, coverage) + self.assertEqual(False, self.core.mixer.get_mute()) def test_import_invalid_type(self): with self.assertRaises(TypeError): From 3251722a289b4803dd161bfc4c219b1a41e29e8a Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 31 Mar 2016 21:55:32 +0200 Subject: [PATCH 31/50] Rename storage.save() to storage.dump(). The name matches the load/dump in the json library. Import olny storage from mopidy.internal (not all of mopidy.internal). --- mopidy/core/actor.py | 2 +- mopidy/internal/storage.py | 2 +- mopidy/local/json.py | 6 +++--- tests/core/test_actor.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 56fe6e74..7b457ac1 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -184,7 +184,7 @@ class Core( history=self.history._export_state(), playback=self.playback._export_state(), mixer=self.mixer._export_state()) - storage.save(file_name, data) + storage.dump(file_name, data) logger.debug('Save state done.') def _load_state(self, coverage): diff --git a/mopidy/internal/storage.py b/mopidy/internal/storage.py index faa53d57..660b22ae 100644 --- a/mopidy/internal/storage.py +++ b/mopidy/internal/storage.py @@ -35,7 +35,7 @@ def load(path): return {} -def save(path, data): +def dump(path, data): """ Serialize data to file. diff --git a/mopidy/local/json.py b/mopidy/local/json.py index 378cab75..1a0307e5 100644 --- a/mopidy/local/json.py +++ b/mopidy/local/json.py @@ -9,7 +9,7 @@ import sys import mopidy from mopidy import compat, local, models -from mopidy import internal +from mopidy.internal import storage as internal_storage from mopidy.internal import timer from mopidy.local import search, storage, translator @@ -99,7 +99,7 @@ class JsonLibrary(local.Library): self._json_file) self._tracks = {} else: - library = internal.storage.load(self._json_file) + library = internal_storage.load(self._json_file) self._tracks = dict((t.uri, t) for t in library.get('tracks', [])) with timer.time_logger('Building browse cache'): @@ -167,7 +167,7 @@ class JsonLibrary(local.Library): self._tracks.pop(uri, None) def close(self): - internal.storage.save(self._json_file, + internal_storage.dump(self._json_file, {'version': mopidy.__version__, 'tracks': self._tracks.values()}) diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index 9d71fcef..a83ae93e 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -81,7 +81,7 @@ class CoreActorExportRestoreTest(unittest.TestCase): file_path = os.path.join(self.temp_dir, 'core', 'state.json.gz') data = {} - storage.save(file_path, data) + storage.dump(file_path, data) self.assertTrue(os.path.isfile(file_path), 'missing persistent file') self.core.setup().get() From 67044cecabe326146377955cd6126fbbb345080a Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 31 Mar 2016 22:07:53 +0200 Subject: [PATCH 32/50] Remove 'TODO: check if playable' We don't need to as each backend a title is still playable. The backend must be able to handle unplayable title. Replace for loop with arrar.extend() --- mopidy/core/tracklist.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index c6dbda6a..bff0190e 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -667,13 +667,7 @@ class TracklistController(object): self.set_repeat(state.repeat) self.set_single(state.single) if 'tracklist' in coverage: - if state.next_tlid > self._next_tlid: - self._next_tlid = state.next_tlid + self._next_tlid = max(state.next_tlid, self._next_tlid) self._tl_tracks = [] - for track in state.tl_tracks: - # TODO: check if any backend will play the track. - # Could be an issue with music streaming services - # (login), disabled extensions and automatically - # generated playlists (pandora). - self._tl_tracks.append(track) + self._tl_tracks.extend(state.tl_tracks) self._increase_version() From 01201d142c2a600f87a573d5b3fca7cea7775ce0 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 31 Mar 2016 23:17:29 +0200 Subject: [PATCH 33/50] Renamed member in HistoryController - add 'play-always' and 'play-last' to docstring for _load_state. - renamed _start_at_pos to _start_at_position - changed 'if const == value:' to 'if value == const:' - changed info text to 'Limiting history' --- mopidy/core/actor.py | 3 ++- mopidy/core/history.py | 5 ++--- mopidy/core/playback.py | 15 +++++++-------- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 7b457ac1..e6c76789 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -197,7 +197,8 @@ class Core( - 'tracklist' fill the tracklist - 'mode' set tracklist properties (consume, random, repeat, single) - - 'autoplay' start playing ('tracklist' also required) + - 'play-last' restore play state ('tracklist' also required) + - 'play-always' start playing ('tracklist' also required) - 'mixer' set mixer volume and mute state - 'history' restore history diff --git a/mopidy/core/history.py b/mopidy/core/history.py index a6b4c817..14d847b3 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -70,7 +70,7 @@ class HistoryController(object): timestamp=timestamp, track=track)) count += 1 if count_max < count: - logger.info('Limit history to %s tracks.', count_max) + logger.info('Limiting history to %s tracks.', count_max) break return HistoryState(history=history_list) @@ -79,5 +79,4 @@ class HistoryController(object): if state: if 'history' in coverage: self._history = [] - for htrack in state.history: - self._history.append((htrack.timestamp, htrack.track)) + self._history = [(h.timestamp, h.track) for h in state.history] diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index b569aebf..3df18b0b 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -29,7 +29,7 @@ class PlaybackController(object): self._last_position = None self._previous = False - self._start_at_pos = None + self._start_at_position = None self._start_paused = False if self._audio: @@ -229,9 +229,9 @@ class PlaybackController(object): self.set_state(PlaybackState.PLAYING) self._trigger_track_playback_started() seek_ok = False - if self._start_at_pos: - seek_ok = self.seek(self._start_at_pos) - self._start_at_pos = None + if self._start_at_position: + seek_ok = self.seek(self._start_at_position) + self._start_at_position = None if not seek_ok and self._start_paused: self.pause() self._start_paused = False @@ -615,9 +615,8 @@ class PlaybackController(object): if 'play-last' in coverage: new_state = state.state if state.tlid is not None: - if PlaybackState.PAUSED == new_state: + if new_state == PlaybackState.PAUSED: self._start_paused = True - if (PlaybackState.PLAYING == new_state or - PlaybackState.PAUSED == new_state): - self._start_at_pos = state.position + if new_state in (PlaybackState.PLAYING, PlaybackState.PAUSED): + self._start_at_position = state.position self.play(tlid=state.tlid) From 44b7974f7919c6f9802d664c68c4662149eae960 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 31 Mar 2016 23:20:26 +0200 Subject: [PATCH 34/50] Restore mixer.volume/mute before starting playback --- 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 e6c76789..77bfdd67 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -220,8 +220,8 @@ class Core( self.history._restore_state(core_state.history, coverage) self.tracklist._restore_state(core_state.tracklist, coverage) # playback after tracklist - self.playback._restore_state(core_state.playback, coverage) self.mixer._restore_state(core_state.mixer, coverage) + self.playback._restore_state(core_state.playback, coverage) logger.debug('Load state done.') From e9e9646d61b5e46c9424c1b0f6eb3944b0e750fa Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 31 Mar 2016 23:24:25 +0200 Subject: [PATCH 35/50] Removed unnecessary docstrings. --- mopidy/core/history.py | 3 --- mopidy/core/mixer.py | 2 -- mopidy/core/playback.py | 2 -- mopidy/core/tracklist.py | 2 -- 4 files changed, 9 deletions(-) diff --git a/mopidy/core/history.py b/mopidy/core/history.py index 14d847b3..d0c936ee 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -59,8 +59,6 @@ class HistoryController(object): return copy.copy(self._history) def _export_state(self): - """Internal method for :class:`mopidy.Core`.""" - # 500 tracks a 3 minutes -> 24 hours history count_max = 500 count = 1 @@ -75,7 +73,6 @@ class HistoryController(object): return HistoryState(history=history_list) def _restore_state(self, state, coverage): - """Internal method for :class:`mopidy.Core`.""" if state: if 'history' in coverage: self._history = [] diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py index 3d9b7003..20e1fb56 100644 --- a/mopidy/core/mixer.py +++ b/mopidy/core/mixer.py @@ -102,12 +102,10 @@ class MixerController(object): return False def _export_state(self): - """Internal method for :class:`mopidy.Core`.""" return MixerState(volume=self.get_volume(), mute=self.get_mute()) def _restore_state(self, state, coverage): - """Internal method for :class:`mopidy.Core`.""" if state: if 'mixer' in coverage: if state.volume: diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 3df18b0b..7d597d71 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -600,14 +600,12 @@ class PlaybackController(object): listener.CoreListener.send('seeked', time_position=time_position) def _export_state(self): - """Internal method for :class:`mopidy.Core`.""" return models.PlaybackState( tlid=self.get_current_tlid(), position=self.get_time_position(), state=self.get_state()) def _restore_state(self, state, coverage): - """Internal method for :class:`mopidy.Core`.""" if state: new_state = None if 'play-always' in coverage: diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index bff0190e..61183438 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -649,7 +649,6 @@ class TracklistController(object): listener.CoreListener.send('options_changed') def _export_state(self): - """Internal method for :class:`mopidy.Core`.""" return TracklistState( tl_tracks=self._tl_tracks, next_tlid=self._next_tlid, @@ -659,7 +658,6 @@ class TracklistController(object): single=self.get_single()) def _restore_state(self, state, coverage): - """Internal method for :class:`mopidy.Core`.""" if state: if 'mode' in coverage: self.set_consume(state.consume) From eb930a1679d8bcb12337afc9b4c1728917a66b4b Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 31 Mar 2016 23:34:57 +0200 Subject: [PATCH 36/50] Catch exception when deleting persistent file. --- mopidy/core/actor.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 77bfdd67..de6c64d0 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -211,8 +211,11 @@ class Core( data = storage.load(file_name) - # Try only once. If something goes wrong, the next start is clean. - os.remove(file_name) + try: + # Try only once. If something goes wrong, the next start is clean. + os.remove(file_name) + except OSError: + logger.info('Failed to delete %s', file_name) if 'state' in data: core_state = data['state'] From cf5dfccee225b9bade6ae438bd3b1cd05b20b39f Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Fri, 1 Apr 2016 19:13:26 +0200 Subject: [PATCH 37/50] Rename PlaybackState.position to time_position. --- mopidy/core/playback.py | 4 ++-- mopidy/internal/models.py | 6 +++--- tests/core/test_playback.py | 6 +++--- tests/internal/test_models.py | 12 ++++++------ 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 7d597d71..ed6acc4b 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -602,7 +602,7 @@ class PlaybackController(object): def _export_state(self): return models.PlaybackState( tlid=self.get_current_tlid(), - position=self.get_time_position(), + time_position=self.get_time_position(), state=self.get_state()) def _restore_state(self, state, coverage): @@ -616,5 +616,5 @@ class PlaybackController(object): if new_state == PlaybackState.PAUSED: self._start_paused = True if new_state in (PlaybackState.PLAYING, PlaybackState.PAUSED): - self._start_at_position = state.position + self._start_at_position = state.time_position self.play(tlid=state.tlid) diff --git a/mopidy/internal/models.py b/mopidy/internal/models.py index b66e0504..7d0ed6b1 100644 --- a/mopidy/internal/models.py +++ b/mopidy/internal/models.py @@ -60,8 +60,8 @@ class PlaybackState(ValidatedImmutableObject): :param tlid: current track tlid :type tlid: int - :param position: play position - :type position: int + :param time_position: play position + :type time_position: int :param state: playback state :type state: :class:`validation.PLAYBACK_STATES` """ @@ -70,7 +70,7 @@ class PlaybackState(ValidatedImmutableObject): tlid = fields.Integer(min=1) # The playback position. Read-only. - position = fields.Integer(min=0) + time_position = fields.Integer(min=0) # The playback state. Read-only. state = fields.Field(choices=validation.PLAYBACK_STATES) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 7a130bad..909e999c 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -1141,7 +1141,7 @@ class TesetCorePlaybackExportRestore(BaseTest): self.replay_events() state = PlaybackState( - position=0, state='playing', tlid=tl_tracks[1].tlid) + time_position=0, state='playing', tlid=tl_tracks[1].tlid) value = self.core.playback._export_state() self.assertEqual(state, value) @@ -1154,7 +1154,7 @@ class TesetCorePlaybackExportRestore(BaseTest): self.assertEqual('stopped', self.core.playback.get_state()) state = PlaybackState( - position=0, state='playing', tlid=tl_tracks[2].tlid) + time_position=0, state='playing', tlid=tl_tracks[2].tlid) coverage = ['play-always'] self.core.playback._restore_state(state, coverage) self.replay_events() @@ -1171,7 +1171,7 @@ class TesetCorePlaybackExportRestore(BaseTest): self.assertEqual('stopped', self.core.playback.get_state()) state = PlaybackState( - position=0, state='playing', tlid=tl_tracks[2].tlid) + time_position=0, state='playing', tlid=tl_tracks[2].tlid) coverage = ['other'] self.core.playback._restore_state(state, coverage) self.replay_events() diff --git a/tests/internal/test_models.py b/tests/internal/test_models.py index 9c181bdd..f780ab98 100644 --- a/tests/internal/test_models.py +++ b/tests/internal/test_models.py @@ -78,16 +78,16 @@ class MixerStateTest(unittest.TestCase): class PlaybackStateTest(unittest.TestCase): def test_position(self): - position = 123456 - result = PlaybackState(position=position) - self.assertEqual(result.position, position) + time_position = 123456 + result = PlaybackState(time_position=time_position) + self.assertEqual(result.time_position, time_position) with self.assertRaises(AttributeError): - result.position = None + result.time_position = None def test_position_invalid(self): - position = -1 + time_position = -1 with self.assertRaises(ValueError): - PlaybackState(position=position) + PlaybackState(time_position=time_position) def test_tl_track(self): tlid = 42 From e9db19a3526307cdffe362569ee864033ae2dea6 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Fri, 1 Apr 2016 19:39:48 +0200 Subject: [PATCH 38/50] Handle all path- and file-names as bytestings --- mopidy/core/actor.py | 4 ++-- mopidy/internal/storage.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index de6c64d0..32b5f47f 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -174,7 +174,7 @@ class Core( Save current state to disk. """ - file_name = os.path.join(self._get_data_dir(), b'state.json.gz') + file_name = bytes(os.path.join(self._get_data_dir(), b'state.json.gz')) logger.info('Save state to %s', file_name) data = {} @@ -206,7 +206,7 @@ class Core( :type coverage: list of string (see above) """ - file_name = os.path.join(self._get_data_dir(), b'state.json.gz') + file_name = bytes(os.path.join(self._get_data_dir(), b'state.json.gz')) logger.info('Load state from %s', file_name) data = storage.load(file_name) diff --git a/mopidy/internal/storage.py b/mopidy/internal/storage.py index 660b22ae..3b7106a0 100644 --- a/mopidy/internal/storage.py +++ b/mopidy/internal/storage.py @@ -17,7 +17,7 @@ def load(path): Deserialize data from file. :param path: full path to import file - :type path: str + :type path: bytes :return: deserialized data :rtype: dict """ @@ -40,7 +40,7 @@ def dump(path, data): Serialize data to file. :param path: full path to export file - :type path: str + :type path: bytes :param data: dictionary containing data to save :type data: dict """ From 6e33bbcadd1a6101b30a120d19c01e40d76d5590 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Fri, 1 Apr 2016 19:50:37 +0200 Subject: [PATCH 39/50] Replace restore/export with load/save Rename _restore_state to _load_state Rename _export_state to _save_state --- mopidy/core/actor.py | 16 ++++++++-------- mopidy/core/history.py | 4 ++-- mopidy/core/mixer.py | 4 ++-- mopidy/core/playback.py | 4 ++-- mopidy/core/tracklist.py | 4 ++-- tests/core/test_actor.py | 4 ++-- tests/core/test_history.py | 8 ++++---- tests/core/test_mixer.py | 16 ++++++++-------- tests/core/test_playback.py | 10 +++++----- tests/core/test_tracklist.py | 12 ++++++------ 10 files changed, 41 insertions(+), 41 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 32b5f47f..bd77b37e 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -180,10 +180,10 @@ class Core( data = {} data['version'] = mopidy.__version__ data['state'] = CoreState( - tracklist=self.tracklist._export_state(), - history=self.history._export_state(), - playback=self.playback._export_state(), - mixer=self.mixer._export_state()) + tracklist=self.tracklist._save_state(), + history=self.history._save_state(), + playback=self.playback._save_state(), + mixer=self.mixer._save_state()) storage.dump(file_name, data) logger.debug('Save state done.') @@ -220,11 +220,11 @@ class Core( if 'state' in data: core_state = data['state'] validation.check_instance(core_state, CoreState) - self.history._restore_state(core_state.history, coverage) - self.tracklist._restore_state(core_state.tracklist, coverage) + self.history._load_state(core_state.history, coverage) + self.tracklist._load_state(core_state.tracklist, coverage) + self.mixer._load_state(core_state.mixer, coverage) # playback after tracklist - self.mixer._restore_state(core_state.mixer, coverage) - self.playback._restore_state(core_state.playback, coverage) + self.playback._load_state(core_state.playback, coverage) logger.debug('Load state done.') diff --git a/mopidy/core/history.py b/mopidy/core/history.py index d0c936ee..da688980 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -58,7 +58,7 @@ class HistoryController(object): """ return copy.copy(self._history) - def _export_state(self): + def _save_state(self): # 500 tracks a 3 minutes -> 24 hours history count_max = 500 count = 1 @@ -72,7 +72,7 @@ class HistoryController(object): break return HistoryState(history=history_list) - def _restore_state(self, state, coverage): + def _load_state(self, state, coverage): if state: if 'history' in coverage: self._history = [] diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py index 20e1fb56..15fafad1 100644 --- a/mopidy/core/mixer.py +++ b/mopidy/core/mixer.py @@ -101,11 +101,11 @@ class MixerController(object): return False - def _export_state(self): + def _save_state(self): return MixerState(volume=self.get_volume(), mute=self.get_mute()) - def _restore_state(self, state, coverage): + def _load_state(self, state, coverage): if state: if 'mixer' in coverage: if state.volume: diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index ed6acc4b..0dd73714 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -599,13 +599,13 @@ class PlaybackController(object): logger.debug('Triggering seeked event') listener.CoreListener.send('seeked', time_position=time_position) - def _export_state(self): + def _save_state(self): return models.PlaybackState( tlid=self.get_current_tlid(), time_position=self.get_time_position(), state=self.get_state()) - def _restore_state(self, state, coverage): + def _load_state(self, state, coverage): if state: new_state = None if 'play-always' in coverage: diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 61183438..2700bef5 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -648,7 +648,7 @@ class TracklistController(object): logger.debug('Triggering options changed event') listener.CoreListener.send('options_changed') - def _export_state(self): + def _save_state(self): return TracklistState( tl_tracks=self._tl_tracks, next_tlid=self._next_tlid, @@ -657,7 +657,7 @@ class TracklistController(object): repeat=self.get_repeat(), single=self.get_single()) - def _restore_state(self, state, coverage): + def _load_state(self, state, coverage): if state: if 'mode' in coverage: self.set_consume(state.consume) diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index a83ae93e..a22ed642 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -69,11 +69,11 @@ class CoreActorExportRestoreTest(unittest.TestCase): pykka.ActorRegistry.stop_all() shutil.rmtree(self.temp_dir) - def test_export_state(self): + def test_save_state(self): self.core.teardown().get() # TODO: implement meaningful test - def test_restore_state(self): + def test_load_state(self): self.core.setup().get() # TODO: implement meaningful test diff --git a/tests/core/test_history.py b/tests/core/test_history.py index 65babde8..84f8b1b1 100644 --- a/tests/core/test_history.py +++ b/tests/core/test_history.py @@ -68,7 +68,7 @@ class CoreHistoryExportRestoreTest(unittest.TestCase): self.history._add_track(self.tracks[2]) self.history._add_track(self.tracks[1]) - value = self.history._export_state() + value = self.history._save_state() self.assertEqual(len(value.history), 2) # last in, first out @@ -81,7 +81,7 @@ class CoreHistoryExportRestoreTest(unittest.TestCase): HistoryTrack(timestamp=45, track=self.refs[2]), HistoryTrack(timestamp=56, track=self.refs[1])]) coverage = ['history'] - self.history._restore_state(state, coverage) + self.history._load_state(state, coverage) hist = self.history.get_history() self.assertEqual(len(hist), 3) @@ -100,7 +100,7 @@ class CoreHistoryExportRestoreTest(unittest.TestCase): def test_import_invalid_type(self): with self.assertRaises(TypeError): - self.history._restore_state(11, None) + self.history._load_state(11, None) def test_import_none(self): - self.history._restore_state(None, None) + self.history._load_state(None, None) diff --git a/tests/core/test_mixer.py b/tests/core/test_mixer.py index 17ecdfb5..be4b314c 100644 --- a/tests/core/test_mixer.py +++ b/tests/core/test_mixer.py @@ -169,7 +169,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): target = MixerState(volume=volume, mute=mute) self.core.mixer.set_volume(volume) self.core.mixer.set_mute(mute) - value = self.core.mixer._export_state() + value = self.core.mixer._save_state() self.assertEqual(target, value) def test_export_unmute(self): @@ -178,7 +178,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): target = MixerState(volume=volume, mute=mute) self.core.mixer.set_volume(volume) self.core.mixer.set_mute(mute) - value = self.core.mixer._export_state() + value = self.core.mixer._save_state() self.assertEqual(target, value) def test_import(self): @@ -186,7 +186,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): volume = 45 target = MixerState(volume=volume) coverage = ['mixer'] - self.core.mixer._restore_state(target, coverage) + self.core.mixer._load_state(target, coverage) self.assertEqual(volume, self.core.mixer.get_volume()) def test_import_not_covered(self): @@ -194,7 +194,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): self.core.mixer.set_mute(True) target = MixerState(volume=56, mute=False) coverage = ['other'] - self.core.mixer._restore_state(target, coverage) + self.core.mixer._load_state(target, coverage) self.assertEqual(21, self.core.mixer.get_volume()) self.assertEqual(True, self.core.mixer.get_mute()) @@ -203,7 +203,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): self.assertEqual(False, self.core.mixer.get_mute()) target = MixerState(mute=True) coverage = ['mixer'] - self.core.mixer._restore_state(target, coverage) + self.core.mixer._load_state(target, coverage) self.assertEqual(True, self.core.mixer.get_mute()) def test_import_mute_off(self): @@ -211,12 +211,12 @@ class CoreMixerExportRestoreTest(unittest.TestCase): self.assertEqual(True, self.core.mixer.get_mute()) target = MixerState(mute=False) coverage = ['mixer'] - self.core.mixer._restore_state(target, coverage) + self.core.mixer._load_state(target, coverage) self.assertEqual(False, self.core.mixer.get_mute()) def test_import_invalid_type(self): with self.assertRaises(TypeError): - self.core.mixer._restore_state(11, None) + self.core.mixer._load_state(11, None) def test_import_none(self): - self.core.mixer._restore_state(None, None) + self.core.mixer._load_state(None, None) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 909e999c..3f4c8fdc 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -1142,7 +1142,7 @@ class TesetCorePlaybackExportRestore(BaseTest): state = PlaybackState( time_position=0, state='playing', tlid=tl_tracks[1].tlid) - value = self.core.playback._export_state() + value = self.core.playback._save_state() self.assertEqual(state, value) @@ -1156,7 +1156,7 @@ class TesetCorePlaybackExportRestore(BaseTest): state = PlaybackState( time_position=0, state='playing', tlid=tl_tracks[2].tlid) coverage = ['play-always'] - self.core.playback._restore_state(state, coverage) + self.core.playback._load_state(state, coverage) self.replay_events() self.assertEqual('playing', self.core.playback.get_state()) @@ -1173,7 +1173,7 @@ class TesetCorePlaybackExportRestore(BaseTest): state = PlaybackState( time_position=0, state='playing', tlid=tl_tracks[2].tlid) coverage = ['other'] - self.core.playback._restore_state(state, coverage) + self.core.playback._load_state(state, coverage) self.replay_events() self.assertEqual('stopped', self.core.playback.get_state()) @@ -1182,10 +1182,10 @@ class TesetCorePlaybackExportRestore(BaseTest): def test_import_invalid_type(self): with self.assertRaises(TypeError): - self.core.playback._restore_state(11, None) + self.core.playback._load_state(11, None) def test_import_none(self): - self.core.playback._restore_state(None, None) + self.core.playback._load_state(None, None) class TestBug1352Regression(BaseTest): diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 5e9170b2..4cd13d1e 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -223,7 +223,7 @@ class TracklistExportRestoreTest(unittest.TestCase): random=False, next_tlid=next_tlid, tl_tracks=tl_tracks) - value = self.core.tracklist._export_state() + value = self.core.tracklist._save_state() self.assertEqual(target, value) def test_import(self): @@ -235,7 +235,7 @@ class TracklistExportRestoreTest(unittest.TestCase): next_tlid=12, tl_tracks=self.tl_tracks) coverage = ['mode', 'tracklist'] - self.core.tracklist._restore_state(target, coverage) + self.core.tracklist._load_state(target, coverage) self.assertEqual(False, self.core.tracklist.get_consume()) self.assertEqual(True, self.core.tracklist.get_repeat()) self.assertEqual(True, self.core.tracklist.get_single()) @@ -259,7 +259,7 @@ class TracklistExportRestoreTest(unittest.TestCase): next_tlid=12, tl_tracks=self.tl_tracks) coverage = ['mode'] - self.core.tracklist._restore_state(target, coverage) + self.core.tracklist._load_state(target, coverage) self.assertEqual(False, self.core.tracklist.get_consume()) self.assertEqual(True, self.core.tracklist.get_repeat()) self.assertEqual(True, self.core.tracklist.get_single()) @@ -278,7 +278,7 @@ class TracklistExportRestoreTest(unittest.TestCase): next_tlid=12, tl_tracks=self.tl_tracks) coverage = ['tracklist'] - self.core.tracklist._restore_state(target, coverage) + self.core.tracklist._load_state(target, coverage) self.assertEqual(False, self.core.tracklist.get_consume()) self.assertEqual(False, self.core.tracklist.get_repeat()) self.assertEqual(False, self.core.tracklist.get_single()) @@ -290,7 +290,7 @@ class TracklistExportRestoreTest(unittest.TestCase): def test_import_invalid_type(self): with self.assertRaises(TypeError): - self.core.tracklist._restore_state(11, None) + self.core.tracklist._load_state(11, None) def test_import_none(self): - self.core.tracklist._restore_state(None, None) + self.core.tracklist._load_state(None, None) From 6ee36752bd02aa3376bde76d30121a0fb9a42e41 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Fri, 1 Apr 2016 22:16:05 +0200 Subject: [PATCH 40/50] Test whole save/load state. --- tests/core/test_actor.py | 94 +++++++++++++++++++++++++++++++++------- 1 file changed, 79 insertions(+), 15 deletions(-) diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index a22ed642..62c0ba2b 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -9,8 +9,12 @@ import mock import pykka +import mopidy + from mopidy.core import Core -from mopidy.internal import storage, versioning +from mopidy.internal import models, storage, versioning +from mopidy.models import Track +from tests import dummy_mixer class CoreActorTest(unittest.TestCase): @@ -52,6 +56,8 @@ class CoreActorExportRestoreTest(unittest.TestCase): def setUp(self): self.temp_dir = tempfile.mkdtemp() + self.state_file = os.path.join(self.temp_dir, 'core', 'state.json.gz') + config = { 'core': { 'max_tracklist_length': 10000, @@ -62,29 +68,87 @@ class CoreActorExportRestoreTest(unittest.TestCase): os.mkdir(os.path.join(self.temp_dir, 'core')) - self.core = Core.start( - config=config, mixer=None, backends=[]).proxy() + self.mixer = dummy_mixer.create_proxy() + self.core = Core( + config=config, mixer=self.mixer, backends=[]) def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() shutil.rmtree(self.temp_dir) def test_save_state(self): - self.core.teardown().get() - # TODO: implement meaningful test + self.core.teardown() - def test_load_state(self): - self.core.setup().get() - # TODO: implement meaningful test + assert os.path.isfile(self.state_file) + reload_data = storage.load(self.state_file) + data = {} + data['version'] = mopidy.__version__ + data['state'] = models.CoreState( + tracklist=models.TracklistState( + repeat=False, random=False, + consume=False, single=False, + next_tlid=1), + history=models.HistoryState(), + playback=models.PlaybackState(state='stopped', + time_position=0), + mixer=models.MixerState()) + assert data == reload_data + + def test_load_state_no_file(self): + self.core.setup() + + assert self.core.mixer.get_mute() is None + assert self.core.mixer.get_volume() is None + assert self.core.tracklist._next_tlid == 1 + assert self.core.tracklist.get_repeat() is False + assert self.core.tracklist.get_random() is False + assert self.core.tracklist.get_consume() is False + assert self.core.tracklist.get_single() is False + assert self.core.tracklist.get_length() == 0 + assert self.core.playback._start_paused is False + assert self.core.playback._start_at_position is None + assert self.core.history.get_length() == 0 + + def test_load_state_with_data(self): + data = {} + data['version'] = mopidy.__version__ + data['state'] = models.CoreState( + tracklist=models.TracklistState( + repeat=True, random=True, + consume=False, single=False, + tl_tracks=[models.TlTrack(tlid=12, track=Track(uri='a:a'))], + next_tlid=14), + history=models.HistoryState(history=[ + models.HistoryTrack( + timestamp=12, + track=models.Ref.track(uri='a:a', name='a')), + models.HistoryTrack( + timestamp=13, + track=models.Ref.track(uri='a:b', name='b'))]), + playback=models.PlaybackState(tlid=12, state='paused', + time_position=432), + mixer=models.MixerState(mute=True, volume=12)) + storage.dump(self.state_file, data) + + self.core.setup() + + assert self.core.mixer.get_mute() is True + assert self.core.mixer.get_volume() == 12 + assert self.core.tracklist._next_tlid == 14 + assert self.core.tracklist.get_repeat() is True + assert self.core.tracklist.get_random() is True + assert self.core.tracklist.get_consume() is False + assert self.core.tracklist.get_single() is False + assert self.core.tracklist.get_length() == 1 + assert self.core.playback._start_paused is True + assert self.core.playback._start_at_position == 432 + assert self.core.history.get_length() == 2 def test_delete_state_file_on_restore(self): - file_path = os.path.join(self.temp_dir, 'core', 'state.json.gz') - data = {} - storage.dump(file_path, data) - self.assertTrue(os.path.isfile(file_path), 'missing persistent file') + storage.dump(self.state_file, data) + assert os.path.isfile(self.state_file) - self.core.setup().get() + self.core.setup() - self.assertFalse(os.path.isfile(file_path), - 'persistent file has to be deleted') + assert not os.path.isfile(self.state_file) From ac47d254a3a12301ef1f53da559e0684dae06beb Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 2 Apr 2016 15:58:05 +0200 Subject: [PATCH 41/50] Remove dead code for 'play-always' --- mopidy/core/actor.py | 1 - mopidy/core/playback.py | 2 -- tests/core/test_playback.py | 2 +- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index bd77b37e..fba11d82 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -198,7 +198,6 @@ class Core( - 'tracklist' fill the tracklist - 'mode' set tracklist properties (consume, random, repeat, single) - 'play-last' restore play state ('tracklist' also required) - - 'play-always' start playing ('tracklist' also required) - 'mixer' set mixer volume and mute state - 'history' restore history diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 0dd73714..aa5c8e01 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -608,8 +608,6 @@ class PlaybackController(object): def _load_state(self, state, coverage): if state: new_state = None - if 'play-always' in coverage: - new_state = PlaybackState.PLAYING if 'play-last' in coverage: new_state = state.state if state.tlid is not None: diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 3f4c8fdc..e23c168e 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -1155,7 +1155,7 @@ class TesetCorePlaybackExportRestore(BaseTest): state = PlaybackState( time_position=0, state='playing', tlid=tl_tracks[2].tlid) - coverage = ['play-always'] + coverage = ['play-last'] self.core.playback._load_state(state, coverage) self.replay_events() From a6e33e537f88a6e810b703992e28aa6571529955 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 2 Apr 2016 16:01:09 +0200 Subject: [PATCH 42/50] Correct wrong docstring --- mopidy/internal/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/internal/models.py b/mopidy/internal/models.py index 7d0ed6b1..11d09bf7 100644 --- a/mopidy/internal/models.py +++ b/mopidy/internal/models.py @@ -42,7 +42,7 @@ class MixerState(ValidatedImmutableObject): :param volume: the volume :type volume: int - :param mute: the volume + :param mute: the mute state :type mute: int """ From d93cc1b44dcd35255cd95e848b7f99ac314a8288 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 2 Apr 2016 16:04:45 +0200 Subject: [PATCH 43/50] Remove unnecessary array initialzation --- mopidy/core/history.py | 1 - 1 file changed, 1 deletion(-) diff --git a/mopidy/core/history.py b/mopidy/core/history.py index da688980..7f7e4fe9 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -75,5 +75,4 @@ class HistoryController(object): def _load_state(self, state, coverage): if state: if 'history' in coverage: - self._history = [] self._history = [(h.timestamp, h.track) for h in state.history] From 00c47117d56534952de22d810add8e668bf93d6a Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 2 Apr 2016 16:11:47 +0200 Subject: [PATCH 44/50] Test models.MixerState.mute --- tests/internal/test_models.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/internal/test_models.py b/tests/internal/test_models.py index f780ab98..eaa638cb 100644 --- a/tests/internal/test_models.py +++ b/tests/internal/test_models.py @@ -68,6 +68,24 @@ class MixerStateTest(unittest.TestCase): with self.assertRaises(ValueError): MixerState(volume=volume) + def test_mute_false(self): + mute = False + result = MixerState(mute=mute) + self.assertEqual(result.mute, mute) + with self.assertRaises(AttributeError): + result.mute = None + + def test_mute_true(self): + mute = True + result = MixerState(mute=mute) + self.assertEqual(result.mute, mute) + with self.assertRaises(AttributeError): + result.mute = False + + def test_mute_default(self): + result = MixerState() + self.assertEqual(result.mute, False) + def test_to_json_and_back(self): result = MixerState(volume=77) serialized = json.dumps(result, cls=ModelJSONEncoder) From b55996da6a4d70dca5dcba9a81fc9371905382f3 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 2 Apr 2016 16:28:04 +0200 Subject: [PATCH 45/50] Changed wording export/restore to save/load --- mopidy/internal/models.py | 10 +++++----- tests/core/test_actor.py | 2 +- tests/core/test_history.py | 10 +++++----- tests/core/test_mixer.py | 18 +++++++++--------- tests/core/test_playback.py | 12 ++++++------ tests/core/test_tracklist.py | 16 ++++++++-------- 6 files changed, 34 insertions(+), 34 deletions(-) diff --git a/mopidy/internal/models.py b/mopidy/internal/models.py index 11d09bf7..67fff40d 100644 --- a/mopidy/internal/models.py +++ b/mopidy/internal/models.py @@ -25,7 +25,7 @@ class HistoryTrack(ValidatedImmutableObject): class HistoryState(ValidatedImmutableObject): """ State of the history controller. - Internally used for import/export of current state. + Internally used for save/load state. :param history: the track history :type history: list of :class:`HistoryTrack` @@ -38,7 +38,7 @@ class HistoryState(ValidatedImmutableObject): class MixerState(ValidatedImmutableObject): """ State of the mixer controller. - Internally used for import/export of current state. + Internally used for save/load state. :param volume: the volume :type volume: int @@ -56,7 +56,7 @@ class MixerState(ValidatedImmutableObject): class PlaybackState(ValidatedImmutableObject): """ State of the playback controller. - Internally used for import/export of current state. + Internally used for save/load state. :param tlid: current track tlid :type tlid: int @@ -80,7 +80,7 @@ class TracklistState(ValidatedImmutableObject): """ State of the tracklist controller. - Internally used for import/export of current state. + Internally used for save/load state. :param repeat: the repeat mode :type repeat: bool @@ -119,7 +119,7 @@ class CoreState(ValidatedImmutableObject): """ State of all Core controller. - Internally used for import/export of current state. + Internally used for save/load state. :param history: State of the history controller :type history: :class:`HistorState` diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index 62c0ba2b..b6669b58 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -52,7 +52,7 @@ class CoreActorTest(unittest.TestCase): self.assertEqual(self.core.version, versioning.get_version()) -class CoreActorExportRestoreTest(unittest.TestCase): +class CoreActorSaveLoadStateTest(unittest.TestCase): def setUp(self): self.temp_dir = tempfile.mkdtemp() diff --git a/tests/core/test_history.py b/tests/core/test_history.py index 84f8b1b1..57cc58ee 100644 --- a/tests/core/test_history.py +++ b/tests/core/test_history.py @@ -49,7 +49,7 @@ class PlaybackHistoryTest(unittest.TestCase): self.assertIn(artist.name, ref.name) -class CoreHistoryExportRestoreTest(unittest.TestCase): +class CoreHistorySaveLoadStateTest(unittest.TestCase): def setUp(self): # noqa: N802 self.tracks = [ @@ -64,7 +64,7 @@ class CoreHistoryExportRestoreTest(unittest.TestCase): self.history = HistoryController() - def test_export(self): + def test_save(self): self.history._add_track(self.tracks[2]) self.history._add_track(self.tracks[1]) @@ -75,7 +75,7 @@ class CoreHistoryExportRestoreTest(unittest.TestCase): self.assertEqual(value.history[0].track, self.refs[1]) self.assertEqual(value.history[1].track, self.refs[2]) - def test_import(self): + def test_load(self): state = HistoryState(history=[ HistoryTrack(timestamp=34, track=self.refs[0]), HistoryTrack(timestamp=45, track=self.refs[2]), @@ -98,9 +98,9 @@ class CoreHistoryExportRestoreTest(unittest.TestCase): self.assertEqual(hist[2], (45, self.refs[2])) self.assertEqual(hist[3], (56, self.refs[1])) - def test_import_invalid_type(self): + def test_load_invalid_type(self): with self.assertRaises(TypeError): self.history._load_state(11, None) - def test_import_none(self): + def test_load_none(self): self.history._load_state(None, None) diff --git a/tests/core/test_mixer.py b/tests/core/test_mixer.py index be4b314c..996b7c23 100644 --- a/tests/core/test_mixer.py +++ b/tests/core/test_mixer.py @@ -157,13 +157,13 @@ class SetMuteBadBackendTest(MockBackendCoreMixerBase): self.assertFalse(self.core.mixer.set_mute(True)) -class CoreMixerExportRestoreTest(unittest.TestCase): +class CoreMixerSaveLoadStateTest(unittest.TestCase): def setUp(self): # noqa: N802 self.mixer = dummy_mixer.create_proxy() self.core = core.Core(mixer=self.mixer, backends=[]) - def test_export_mute(self): + def test_save_mute(self): volume = 32 mute = False target = MixerState(volume=volume, mute=mute) @@ -172,7 +172,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): value = self.core.mixer._save_state() self.assertEqual(target, value) - def test_export_unmute(self): + def test_save_unmute(self): volume = 33 mute = True target = MixerState(volume=volume, mute=mute) @@ -181,7 +181,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): value = self.core.mixer._save_state() self.assertEqual(target, value) - def test_import(self): + def test_load(self): self.core.mixer.set_volume(11) volume = 45 target = MixerState(volume=volume) @@ -189,7 +189,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): self.core.mixer._load_state(target, coverage) self.assertEqual(volume, self.core.mixer.get_volume()) - def test_import_not_covered(self): + def test_load_not_covered(self): self.core.mixer.set_volume(21) self.core.mixer.set_mute(True) target = MixerState(volume=56, mute=False) @@ -198,7 +198,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): self.assertEqual(21, self.core.mixer.get_volume()) self.assertEqual(True, self.core.mixer.get_mute()) - def test_import_mute_on(self): + def test_load_mute_on(self): self.core.mixer.set_mute(False) self.assertEqual(False, self.core.mixer.get_mute()) target = MixerState(mute=True) @@ -206,7 +206,7 @@ class CoreMixerExportRestoreTest(unittest.TestCase): self.core.mixer._load_state(target, coverage) self.assertEqual(True, self.core.mixer.get_mute()) - def test_import_mute_off(self): + def test_load_mute_off(self): self.core.mixer.set_mute(True) self.assertEqual(True, self.core.mixer.get_mute()) target = MixerState(mute=False) @@ -214,9 +214,9 @@ class CoreMixerExportRestoreTest(unittest.TestCase): self.core.mixer._load_state(target, coverage) self.assertEqual(False, self.core.mixer.get_mute()) - def test_import_invalid_type(self): + def test_load_invalid_type(self): with self.assertRaises(TypeError): self.core.mixer._load_state(11, None) - def test_import_none(self): + def test_load_none(self): self.core.mixer._load_state(None, None) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index e23c168e..e63609bf 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -1132,9 +1132,9 @@ class TestBug1177Regression(unittest.TestCase): b.playback.change_track.assert_called_once_with(track2) -class TesetCorePlaybackExportRestore(BaseTest): +class TestCorePlaybackSaveLoadState(BaseTest): - def test_export(self): + def test_save(self): tl_tracks = self.core.tracklist.get_tl_tracks() self.core.playback.play(tl_tracks[1]) @@ -1146,7 +1146,7 @@ class TesetCorePlaybackExportRestore(BaseTest): self.assertEqual(state, value) - def test_import(self): + def test_load(self): tl_tracks = self.core.tracklist.get_tl_tracks() self.core.playback.stop() @@ -1163,7 +1163,7 @@ class TesetCorePlaybackExportRestore(BaseTest): self.assertEqual(tl_tracks[2], self.core.playback.get_current_tl_track()) - def test_import_not_covered(self): + def test_load_not_covered(self): tl_tracks = self.core.tracklist.get_tl_tracks() self.core.playback.stop() @@ -1180,11 +1180,11 @@ class TesetCorePlaybackExportRestore(BaseTest): self.assertEqual(None, self.core.playback.get_current_tl_track()) - def test_import_invalid_type(self): + def test_load_invalid_type(self): with self.assertRaises(TypeError): self.core.playback._load_state(11, None) - def test_import_none(self): + def test_load_none(self): self.core.playback._load_state(None, None) diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 4cd13d1e..120ae1f0 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -180,7 +180,7 @@ class TracklistIndexTest(unittest.TestCase): self.assertEqual(2, self.core.tracklist.index()) -class TracklistExportRestoreTest(unittest.TestCase): +class TracklistSaveLoadStateTest(unittest.TestCase): def setUp(self): # noqa: N802 config = { @@ -211,7 +211,7 @@ class TracklistExportRestoreTest(unittest.TestCase): self.core.playback = mock.Mock(spec=core.PlaybackController) - def test_export(self): + def test_save(self): tl_tracks = self.core.tracklist.add(uris=[ t.uri for t in self.tracks]) consume = True @@ -226,7 +226,7 @@ class TracklistExportRestoreTest(unittest.TestCase): value = self.core.tracklist._save_state() self.assertEqual(target, value) - def test_import(self): + def test_load(self): old_version = self.core.tracklist.get_version() target = TracklistState(consume=False, repeat=True, @@ -245,12 +245,12 @@ class TracklistExportRestoreTest(unittest.TestCase): self.assertEqual(self.tl_tracks, self.core.tracklist.get_tl_tracks()) self.assertGreater(self.core.tracklist.get_version(), old_version) - # after import, adding more tracks must be possible + # after load, adding more tracks must be possible self.core.tracklist.add(uris=[self.tracks[1].uri]) self.assertEqual(13, self.core.tracklist._next_tlid) self.assertEqual(5, self.core.tracklist.get_length()) - def test_import_mode_only(self): + def test_load_mode_only(self): old_version = self.core.tracklist.get_version() target = TracklistState(consume=False, repeat=True, @@ -269,7 +269,7 @@ class TracklistExportRestoreTest(unittest.TestCase): self.assertEqual([], self.core.tracklist.get_tl_tracks()) self.assertEqual(self.core.tracklist.get_version(), old_version) - def test_import_tracklist_only(self): + def test_load_tracklist_only(self): old_version = self.core.tracklist.get_version() target = TracklistState(consume=False, repeat=True, @@ -288,9 +288,9 @@ class TracklistExportRestoreTest(unittest.TestCase): self.assertEqual(self.tl_tracks, self.core.tracklist.get_tl_tracks()) self.assertGreater(self.core.tracklist.get_version(), old_version) - def test_import_invalid_type(self): + def test_load_invalid_type(self): with self.assertRaises(TypeError): self.core.tracklist._load_state(11, None) - def test_import_none(self): + def test_load_none(self): self.core.tracklist._load_state(None, None) From 4693fa7f8ef919d465fe980f089cb57ac43f0018 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 2 Apr 2016 16:55:18 +0200 Subject: [PATCH 46/50] Correct wrong docstring --- mopidy/internal/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/internal/models.py b/mopidy/internal/models.py index 67fff40d..6ff17b5b 100644 --- a/mopidy/internal/models.py +++ b/mopidy/internal/models.py @@ -90,7 +90,7 @@ class TracklistState(ValidatedImmutableObject): :type random: bool :param single: the single mode :type single: bool - :param next_tlid: the id of the next track to play + :param next_tlid: the id for the next added track :type next_tlid: int :param tl_tracks: the list of tracks :type tl_tracks: list of :class:`TlTrack` From 100d7dba7b390a7dc4b557f075598162b27f65fd Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Sat, 2 Apr 2016 17:12:31 +0200 Subject: [PATCH 47/50] Reworked if conditions in _load_state --- mopidy/core/history.py | 5 ++--- mopidy/core/mixer.py | 9 ++++----- mopidy/core/playback.py | 16 ++++++---------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/mopidy/core/history.py b/mopidy/core/history.py index 7f7e4fe9..22adb4a9 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -73,6 +73,5 @@ class HistoryController(object): return HistoryState(history=history_list) def _load_state(self, state, coverage): - if state: - if 'history' in coverage: - self._history = [(h.timestamp, h.track) for h in state.history] + if state and 'history' in coverage: + self._history = [(h.timestamp, h.track) for h in state.history] diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py index 15fafad1..8707c096 100644 --- a/mopidy/core/mixer.py +++ b/mopidy/core/mixer.py @@ -106,8 +106,7 @@ class MixerController(object): mute=self.get_mute()) def _load_state(self, state, coverage): - if state: - if 'mixer' in coverage: - if state.volume: - self.set_volume(state.volume) - self.set_mute(state.mute) + if state and 'mixer' in coverage: + self.set_mute(state.mute) + if state.volume: + self.set_volume(state.volume) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index aa5c8e01..1c809656 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -606,13 +606,9 @@ class PlaybackController(object): state=self.get_state()) def _load_state(self, state, coverage): - if state: - new_state = None - if 'play-last' in coverage: - new_state = state.state - if state.tlid is not None: - if new_state == PlaybackState.PAUSED: - self._start_paused = True - if new_state in (PlaybackState.PLAYING, PlaybackState.PAUSED): - self._start_at_position = state.time_position - self.play(tlid=state.tlid) + if state and 'play-last' in coverage and state.tlid is not None: + if state.state == PlaybackState.PAUSED: + self._start_paused = True + if state.state in (PlaybackState.PLAYING, PlaybackState.PAUSED): + self._start_at_position = state.time_position + self.play(tlid=state.tlid) From 46dd36d91452d00b4c95cb004b5c8c5f83105449 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jens=20L=C3=BCtjen?= Date: Fri, 16 Sep 2016 20:28:10 +0200 Subject: [PATCH 48/50] Optimize _tl_tracks initialization --- mopidy/core/tracklist.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 2700bef5..37930f79 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -666,6 +666,5 @@ class TracklistController(object): self.set_single(state.single) if 'tracklist' in coverage: self._next_tlid = max(state.next_tlid, self._next_tlid) - self._tl_tracks = [] - self._tl_tracks.extend(state.tl_tracks) + self._tl_tracks = list(state.tl_tracks) self._increase_version() From 92767e49f9b51f6cc5d80884c23173668331ed33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jens=20L=C3=BCtjen?= Date: Fri, 16 Sep 2016 21:01:10 +0200 Subject: [PATCH 49/50] Fix typos and formatting --- docs/changelog.rst | 2 +- docs/config.rst | 2 +- mopidy/core/actor.py | 27 +++++++++++++-------------- mopidy/core/history.py | 6 +++--- mopidy/internal/storage.py | 2 +- mopidy/local/json.py | 7 ++++--- tests/core/test_actor.py | 5 +++-- 7 files changed, 26 insertions(+), 25 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5573da6d..851359d8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -11,7 +11,7 @@ v2.1.0 (UNRELEASED) Feature release. - Core: Mopidy restores its last state when started. Can be enabled by setting - the config value :confval:`core/restore_state` to `true`. + the config value :confval:`core/restore_state` to ``true``. - MPD: Fix MPD protocol for ``replay_gain_status`` command. The actual command remains unimplemented. (PR: :issue:`1520`) diff --git a/docs/config.rst b/docs/config.rst index df8e2a7d..5c1257d7 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -115,7 +115,7 @@ Core config section When set to ``true``, Mopidy restores its last state when started. The restored state includes the tracklist, playback history, - the playback state, and the mixers volume and mute state. + the playback state, the volume, and mute state. Default is ``false``. diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index fba11d82..5f7743eb 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -141,7 +141,7 @@ class Core( CoreListener.send('stream_title_changed', title=title) def setup(self): - """ Do not call this function. It is for internal use at startup.""" + """Do not call this function. It is for internal use at startup.""" try: coverage = [] if self._config and 'restore_state' in self._config['core']: @@ -154,7 +154,7 @@ class Core( logger.warn('Restore state: Unexpected error: %s', str(e)) def teardown(self): - """ Do not call this function. It is for internal use at shutdown.""" + """Do not call this function. It is for internal use at shutdown.""" try: if self._config and 'restore_state' in self._config['core']: if self._config['core']['restore_state']: @@ -164,8 +164,7 @@ class Core( def _get_data_dir(self): # get or create data director for core - data_dir_path = bytes( - os.path.join(self._config['core']['data_dir'], 'core')) + data_dir_path = os.path.join(self._config['core']['data_dir'], b'core') path.get_or_create_dir(data_dir_path) return data_dir_path @@ -174,8 +173,8 @@ class Core( Save current state to disk. """ - file_name = bytes(os.path.join(self._get_data_dir(), b'state.json.gz')) - logger.info('Save state to %s', file_name) + file_name = os.path.join(self._get_data_dir(), b'state.json.gz') + logger.info('Saveing state to %s', file_name) data = {} data['version'] = mopidy.__version__ @@ -185,15 +184,15 @@ class Core( playback=self.playback._save_state(), mixer=self.mixer._save_state()) storage.dump(file_name, data) - logger.debug('Save state done.') + logger.debug('Saveing state done') def _load_state(self, coverage): """ Restore state from disk. - Load state from disk and restore it. Parameter `coverage` - limits the amount data to restore. Possible - values for `coverage` (list of one or more of): + Load state from disk and restore it. Parameter ``coverage`` + limits the amount of data to restore. Possible + values for ``coverage`` (list of one or more of): - 'tracklist' fill the tracklist - 'mode' set tracklist properties (consume, random, repeat, single) @@ -202,11 +201,11 @@ class Core( - 'history' restore history :param coverage: amount of data to restore - :type coverage: list of string (see above) + :type coverage: list of strings """ - file_name = bytes(os.path.join(self._get_data_dir(), b'state.json.gz')) - logger.info('Load state from %s', file_name) + file_name = os.path.join(self._get_data_dir(), b'state.json.gz') + logger.info('Loading state from %s', file_name) data = storage.load(file_name) @@ -224,7 +223,7 @@ class Core( self.mixer._load_state(core_state.mixer, coverage) # playback after tracklist self.playback._load_state(core_state.playback, coverage) - logger.debug('Load state done.') + logger.debug('Loading state done') class Backends(list): diff --git a/mopidy/core/history.py b/mopidy/core/history.py index 22adb4a9..94ee6e87 100644 --- a/mopidy/core/history.py +++ b/mopidy/core/history.py @@ -64,11 +64,11 @@ class HistoryController(object): count = 1 history_list = [] for timestamp, track in self._history: - history_list.append(HistoryTrack( - timestamp=timestamp, track=track)) + history_list.append( + HistoryTrack(timestamp=timestamp, track=track)) count += 1 if count_max < count: - logger.info('Limiting history to %s tracks.', count_max) + logger.info('Limiting history to %s tracks', count_max) break return HistoryState(history=history_list) diff --git a/mopidy/internal/storage.py b/mopidy/internal/storage.py index 3b7106a0..6da53a00 100644 --- a/mopidy/internal/storage.py +++ b/mopidy/internal/storage.py @@ -23,7 +23,7 @@ def load(path): """ # Todo: raise an exception in case of error? if not os.path.isfile(path): - logger.info('File does not exist: %s.', path) + logger.info('File does not exist: %s', path) return {} try: with gzip.open(path, 'rb') as fp: diff --git a/mopidy/local/json.py b/mopidy/local/json.py index 1a0307e5..2e39b68b 100644 --- a/mopidy/local/json.py +++ b/mopidy/local/json.py @@ -167,9 +167,10 @@ class JsonLibrary(local.Library): self._tracks.pop(uri, None) def close(self): - internal_storage.dump(self._json_file, - {'version': mopidy.__version__, - 'tracks': self._tracks.values()}) + internal_storage.dump(self._json_file, { + 'version': mopidy.__version__, + 'tracks': self._tracks.values() + }) def clear(self): try: diff --git a/tests/core/test_actor.py b/tests/core/test_actor.py index b6669b58..c5da74d1 100644 --- a/tests/core/test_actor.py +++ b/tests/core/test_actor.py @@ -56,7 +56,8 @@ class CoreActorSaveLoadStateTest(unittest.TestCase): def setUp(self): self.temp_dir = tempfile.mkdtemp() - self.state_file = os.path.join(self.temp_dir, 'core', 'state.json.gz') + self.state_file = os.path.join(self.temp_dir, + b'core', b'state.json.gz') config = { 'core': { @@ -66,7 +67,7 @@ class CoreActorSaveLoadStateTest(unittest.TestCase): } } - os.mkdir(os.path.join(self.temp_dir, 'core')) + os.mkdir(os.path.join(self.temp_dir, b'core')) self.mixer = dummy_mixer.create_proxy() self.core = Core( From b0f749092651dcbb181c1019a0e1960027d73982 Mon Sep 17 00:00:00 2001 From: dublok Date: Sat, 17 Sep 2016 18:22:39 +0200 Subject: [PATCH 50/50] Fix typos --- mopidy/core/actor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 5f7743eb..03efd6a8 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -174,7 +174,7 @@ class Core( """ file_name = os.path.join(self._get_data_dir(), b'state.json.gz') - logger.info('Saveing state to %s', file_name) + logger.info('Saving state to %s', file_name) data = {} data['version'] = mopidy.__version__ @@ -184,7 +184,7 @@ class Core( playback=self.playback._save_state(), mixer=self.mixer._save_state()) storage.dump(file_name, data) - logger.debug('Saveing state done') + logger.debug('Saving state done') def _load_state(self, coverage): """