From 1892e472a0e04097c509737b9f4c53a84b71c0c4 Mon Sep 17 00:00:00 2001 From: Ronald Zielaznicki Date: Fri, 17 Jul 2015 23:06:31 -0400 Subject: [PATCH 1/7] Added config schema --- mopidy/core/ext.conf | 2 ++ mopidy/core/ext.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/mopidy/core/ext.conf b/mopidy/core/ext.conf index 94b2ad54..f186adf0 100644 --- a/mopidy/core/ext.conf +++ b/mopidy/core/ext.conf @@ -1 +1,3 @@ [core] +enabled = true +max_tracklist_length = 10000 diff --git a/mopidy/core/ext.py b/mopidy/core/ext.py index 7a610244..9b758119 100644 --- a/mopidy/core/ext.py +++ b/mopidy/core/ext.py @@ -18,7 +18,8 @@ class Extension(ext.Extension): def get_config_schema(self): schema = super(Extension, self).get_config_schema() - del schema['enabled'] # core cannot be disabled + schema['max_tracklist_length'] = config.Integer( + minimum=1, maximum=10000) return schema def setup(self, registry): From 4614f04c8b3d2b0cb61708e9d4a721a2d3b60322 Mon Sep 17 00:00:00 2001 From: Ronald Zielaznicki Date: Sun, 19 Jul 2015 13:31:43 -0400 Subject: [PATCH 2/7] Added in a CoreError with a TracklistFull subclass --- mopidy/exceptions.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/mopidy/exceptions.py b/mopidy/exceptions.py index d02a288a..3fb2f1c2 100644 --- a/mopidy/exceptions.py +++ b/mopidy/exceptions.py @@ -19,6 +19,13 @@ class MopidyException(Exception): class BackendError(MopidyException): pass + + +class CoreError(MopidyException): + + def __init(self, message, errno=None): + super(CoreError, self).__init(message, errno) + self.errno = errno class ExtensionError(MopidyException): @@ -43,6 +50,13 @@ class MixerError(MopidyException): class ScannerError(MopidyException): pass + +class TracklistFull(CoreError): + + def __init(self, message, errno=None): + super(TracklistFull, self).__init(message, errno) + self.errno = errno + class AudioException(MopidyException): pass From f6f490efc5662333fc8727115d5568dba98a8488 Mon Sep 17 00:00:00 2001 From: Ronald Zielaznicki Date: Sun, 19 Jul 2015 20:23:48 -0400 Subject: [PATCH 3/7] Add a max playlist limit to the tracklist. --- mopidy/core/tracklist.py | 8 ++++++++ mopidy/exceptions.py | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 028e7c02..0b3e55d2 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals import logging import random +from mopidy import exceptions from mopidy.core import listener from mopidy.internal import deprecation, validation from mopidy.models import TlTrack, Track @@ -433,6 +434,13 @@ class TracklistController(object): tl_tracks = [] for track in tracks: + if(self.get_length() >= + self.core._config['core']['max_tracklist_length']): + + raise exceptions.TracklistFull( + 'TracklistFull: Tried to add ' + + 'too many uris to the tracklist.') + break tl_track = TlTrack(self._next_tlid, track) self._next_tlid += 1 if at_position is not None: diff --git a/mopidy/exceptions.py b/mopidy/exceptions.py index 3fb2f1c2..3d25471f 100644 --- a/mopidy/exceptions.py +++ b/mopidy/exceptions.py @@ -19,10 +19,10 @@ class MopidyException(Exception): class BackendError(MopidyException): pass - + class CoreError(MopidyException): - + def __init(self, message, errno=None): super(CoreError, self).__init(message, errno) self.errno = errno @@ -50,9 +50,9 @@ class MixerError(MopidyException): class ScannerError(MopidyException): pass - + class TracklistFull(CoreError): - + def __init(self, message, errno=None): super(TracklistFull, self).__init(message, errno) self.errno = errno From 82ed6607772b50df4033b712038196766efe391c Mon Sep 17 00:00:00 2001 From: Ronald Zielaznicki Date: Sun, 19 Jul 2015 23:05:39 -0400 Subject: [PATCH 4/7] Add core config values to relevent test cases. --- tests/core/test_events.py | 9 +++++++- tests/core/test_playback.py | 41 +++++++++++++++++++++++++++++----- tests/core/test_tracklist.py | 18 ++++++++++++--- tests/local/test_playback.py | 5 ++++- tests/local/test_tracklist.py | 5 ++++- tests/mpd/protocol/__init__.py | 7 +++++- tests/mpd/test_status.py | 10 ++++++++- 7 files changed, 82 insertions(+), 13 deletions(-) diff --git a/tests/core/test_events.py b/tests/core/test_events.py index 7c8eba1d..9a439084 100644 --- a/tests/core/test_events.py +++ b/tests/core/test_events.py @@ -17,12 +17,19 @@ from tests import dummy_backend class BackendEventsTest(unittest.TestCase): def setUp(self): # noqa: N802 + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + self.backend = dummy_backend.create_proxy() self.backend.library.dummy_library = [ Track(uri='dummy:a'), Track(uri='dummy:b')] with deprecation.ignore(): - self.core = core.Core.start(backends=[self.backend]).proxy() + self.core = core.Core.start( + config, backends=[self.backend]).proxy() def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 76054684..c4ba01a6 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -18,6 +18,12 @@ from tests import dummy_audio as audio class CorePlaybackTest(unittest.TestCase): def setUp(self): # noqa: N802 + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + self.backend1 = mock.Mock() self.backend1.uri_schemes.get.return_value = ['dummy1'] self.playback1 = mock.Mock(spec=backend.PlaybackProvider) @@ -46,7 +52,7 @@ class CorePlaybackTest(unittest.TestCase): self.uris = [ 'dummy1:a', 'dummy2:a', 'dummy3:a', 'dummy1:b', 'dummy1:c'] - self.core = core.Core(mixer=None, backends=[ + self.core = core.Core(config, mixer=None, backends=[ self.backend1, self.backend2, self.backend3]) def lookup(uris): @@ -614,9 +620,16 @@ class TestBackend(pykka.ThreadingActor, backend.Backend): class TestStream(unittest.TestCase): def setUp(self): # noqa: N802 + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + self.audio = audio.DummyAudio.start().proxy() self.backend = TestBackend.start(config={}, audio=self.audio).proxy() - self.core = core.Core(audio=self.audio, backends=[self.backend]) + self.core = core.Core( + config, audio=self.audio, backends=[self.backend]) self.playback = self.core.playback self.tracks = [Track(uri='dummy:a', length=1234), @@ -698,6 +711,12 @@ class TestStream(unittest.TestCase): class CorePlaybackWithOldBackendTest(unittest.TestCase): def test_type_error_from_old_backend_does_not_crash_core(self): + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + b = mock.Mock() b.uri_schemes.get.return_value = ['dummy1'] b.playback = mock.Mock(spec=backend.PlaybackProvider) @@ -705,7 +724,7 @@ class CorePlaybackWithOldBackendTest(unittest.TestCase): b.library.lookup.return_value.get.return_value = [ Track(uri='dummy1:a', length=40000)] - c = core.Core(mixer=None, backends=[b]) + c = core.Core(config, mixer=None, backends=[b]) c.tracklist.add(uris=['dummy1:a']) c.playback.play() # No TypeError == test passed. b.playback.play.assert_called_once_with() @@ -714,9 +733,15 @@ class CorePlaybackWithOldBackendTest(unittest.TestCase): class TestPlay(unittest.TestCase): def setUp(self): # noqa: N802 + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + self.backend = mock.Mock() self.backend.uri_schemes.get.return_value = ['dummy'] - self.core = core.Core(backends=[self.backend]) + self.core = core.Core(config, backends=[self.backend]) self.tracks = [Track(uri='dummy:a', length=1234), Track(uri='dummy:b', length=1234)] @@ -732,6 +757,12 @@ class TestPlay(unittest.TestCase): class Bug1177RegressionTest(unittest.TestCase): def test(self): + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + b = mock.Mock() b.uri_schemes.get.return_value = ['dummy'] b.playback = mock.Mock(spec=backend.PlaybackProvider) @@ -741,7 +772,7 @@ class Bug1177RegressionTest(unittest.TestCase): track1 = Track(uri='dummy:a', length=40000) track2 = Track(uri='dummy:b', length=40000) - c = core.Core(mixer=None, backends=[b]) + c = core.Core(config, mixer=None, backends=[b]) c.tracklist.add([track1, track2]) c.playback.play() diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 83b576ea..24edb2e7 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -11,7 +11,13 @@ from mopidy.models import TlTrack, Track class TracklistTest(unittest.TestCase): - def setUp(self): # noqa: N802 + def setUp(self): # noqa: + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + self.tracks = [ Track(uri='dummy1:a', name='foo'), Track(uri='dummy1:b', name='foo'), @@ -29,7 +35,7 @@ class TracklistTest(unittest.TestCase): self.library.lookup.side_effect = lookup self.backend.library = self.library - self.core = core.Core(mixer=None, backends=[self.backend]) + self.core = core.Core(config, mixer=None, backends=[self.backend]) self.tl_tracks = self.core.tracklist.add(uris=[ t.uri for t in self.tracks]) @@ -107,6 +113,12 @@ class TracklistTest(unittest.TestCase): class TracklistIndexTest(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'), @@ -116,7 +128,7 @@ class TracklistIndexTest(unittest.TestCase): def lookup(uris): return {u: [t for t in self.tracks if t.uri == u] for u in uris} - self.core = core.Core(mixer=None, backends=[]) + self.core = core.Core(config, mixer=None, backends=[]) self.core.library = mock.Mock(spec=core.LibraryController) self.core.library.lookup.side_effect = lookup diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index 23e427d9..bab70847 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -22,6 +22,9 @@ from tests.local import generate_song, populate_tracklist class LocalPlaybackProviderTest(unittest.TestCase): config = { + 'core': { + 'max_tracklist_length': 10000, + }, 'local': { 'media_dir': path_to_data_dir(''), 'data_dir': path_to_data_dir(''), @@ -51,7 +54,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.audio = dummy_audio.create_proxy() self.backend = actor.LocalBackend.start( config=self.config, audio=self.audio).proxy() - self.core = core.Core(backends=[self.backend]) + self.core = core.Core(self.config, backends=[self.backend]) self.playback = self.core.playback self.tracklist = self.core.tracklist diff --git a/tests/local/test_tracklist.py b/tests/local/test_tracklist.py index 63ef8fde..72da3f13 100644 --- a/tests/local/test_tracklist.py +++ b/tests/local/test_tracklist.py @@ -17,6 +17,9 @@ from tests.local import generate_song, populate_tracklist class LocalTracklistProviderTest(unittest.TestCase): config = { + 'core': { + 'max_tracklist_length': 10000 + }, 'local': { 'media_dir': path_to_data_dir(''), 'data_dir': path_to_data_dir(''), @@ -35,7 +38,7 @@ class LocalTracklistProviderTest(unittest.TestCase): self.audio = dummy_audio.create_proxy() self.backend = actor.LocalBackend.start( config=self.config, audio=self.audio).proxy() - self.core = core.Core(mixer=None, backends=[self.backend]) + self.core = core.Core(self.config, mixer=None, backends=[self.backend]) self.controller = self.core.tracklist self.playback = self.core.playback diff --git a/tests/mpd/protocol/__init__.py b/tests/mpd/protocol/__init__.py index e66bf88a..754b4418 100644 --- a/tests/mpd/protocol/__init__.py +++ b/tests/mpd/protocol/__init__.py @@ -31,6 +31,9 @@ class BaseTestCase(unittest.TestCase): def get_config(self): return { + 'core': { + 'max_tracklist_length': 10000 + }, 'mpd': { 'password': None, } @@ -45,7 +48,9 @@ class BaseTestCase(unittest.TestCase): with deprecation.ignore(): self.core = core.Core.start( - mixer=self.mixer, backends=[self.backend]).proxy() + self.get_config(), + mixer=self.mixer, + backends=[self.backend]).proxy() self.uri_map = uri_mapper.MpdUriMapper(self.core) self.connection = MockConnection() diff --git a/tests/mpd/test_status.py b/tests/mpd/test_status.py index d36ad4dc..76fa9fcb 100644 --- a/tests/mpd/test_status.py +++ b/tests/mpd/test_status.py @@ -25,12 +25,20 @@ STOPPED = PlaybackState.STOPPED class StatusHandlerTest(unittest.TestCase): def setUp(self): # noqa: N802 + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + self.mixer = dummy_mixer.create_proxy() self.backend = dummy_backend.create_proxy() with deprecation.ignore(): self.core = core.Core.start( - mixer=self.mixer, backends=[self.backend]).proxy() + config, + mixer=self.mixer, + backends=[self.backend]).proxy() self.dispatcher = dispatcher.MpdDispatcher(core=self.core) self.context = self.dispatcher.context From 3aacfd7147836ef95133aa88d558a1d69bbcd0cd Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 20 Jul 2015 16:45:14 +0200 Subject: [PATCH 5/7] exception: Fix typo in new CoreErrors --- mopidy/exceptions.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mopidy/exceptions.py b/mopidy/exceptions.py index 3d25471f..4aa66e63 100644 --- a/mopidy/exceptions.py +++ b/mopidy/exceptions.py @@ -23,8 +23,8 @@ class BackendError(MopidyException): class CoreError(MopidyException): - def __init(self, message, errno=None): - super(CoreError, self).__init(message, errno) + def __init__(self, message, errno=None): + super(CoreError, self).__init__(message, errno) self.errno = errno @@ -53,8 +53,8 @@ class ScannerError(MopidyException): class TracklistFull(CoreError): - def __init(self, message, errno=None): - super(TracklistFull, self).__init(message, errno) + def __init__(self, message, errno=None): + super(TracklistFull, self).__init__(message, errno) self.errno = errno From 8bb29cd28ec99f6feb83f3932372aa6817215356 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 20 Jul 2015 16:53:04 +0200 Subject: [PATCH 6/7] core: Update tracklist full error message --- mopidy/core/tracklist.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 0b3e55d2..63a38eae 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -432,15 +432,13 @@ class TracklistController(object): tracks.extend(track_map[uri]) tl_tracks = [] + max_length = self.core._config['core']['max_tracklist_length'] for track in tracks: - if(self.get_length() >= - self.core._config['core']['max_tracklist_length']): - + if self.get_length() >= max_length: raise exceptions.TracklistFull( - 'TracklistFull: Tried to add ' + - 'too many uris to the tracklist.') - break + 'Tracklist may contain at most %d tracks.' % max_length) + tl_track = TlTrack(self._next_tlid, track) self._next_tlid += 1 if at_position is not None: From 2c30934c2eb03a7544b8e63178be084ddeb2241d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 20 Jul 2015 17:01:40 +0200 Subject: [PATCH 7/7] core: Remove core "extension" as it is not needed for config --- mopidy/config/__init__.py | 4 ++++ mopidy/core/__init__.py | 1 - mopidy/core/ext.py | 26 -------------------------- setup.py | 1 - 4 files changed, 4 insertions(+), 28 deletions(-) delete mode 100644 mopidy/core/ext.py diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 3f1f978c..8d3fa376 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -15,6 +15,10 @@ from mopidy.internal import path, versioning logger = logging.getLogger(__name__) +_core_schema = ConfigSchema('core') +# MPD supports at most 10k tracks, some clients segfault when this is exceeded. +_core_schema['max_tracklist_length'] = Integer(minimum=1, maximum=10000) + _logging_schema = ConfigSchema('logging') _logging_schema['color'] = Boolean() _logging_schema['console_format'] = String() diff --git a/mopidy/core/__init__.py b/mopidy/core/__init__.py index 912856d0..720f9c38 100644 --- a/mopidy/core/__init__.py +++ b/mopidy/core/__init__.py @@ -2,7 +2,6 @@ from __future__ import absolute_import, unicode_literals # flake8: noqa from .actor import Core -from .ext import Extension from .history import HistoryController from .library import LibraryController from .listener import CoreListener diff --git a/mopidy/core/ext.py b/mopidy/core/ext.py deleted file mode 100644 index 9b758119..00000000 --- a/mopidy/core/ext.py +++ /dev/null @@ -1,26 +0,0 @@ -from __future__ import absolute_import, unicode_literals - -import os - -import mopidy -from mopidy import config, ext - - -class Extension(ext.Extension): - - dist_name = 'Mopidy-Core' - ext_name = 'core' - version = mopidy.__version__ - - def get_default_config(self): - conf_file = os.path.join(os.path.dirname(__file__), 'ext.conf') - return config.read(conf_file) - - def get_config_schema(self): - schema = super(Extension, self).get_config_schema() - schema['max_tracklist_length'] = config.Integer( - minimum=1, maximum=10000) - return schema - - def setup(self, registry): - pass # core has nothing to register diff --git a/setup.py b/setup.py index 394431fc..ca121f74 100644 --- a/setup.py +++ b/setup.py @@ -34,7 +34,6 @@ setup( 'mopidy = mopidy.__main__:main', ], 'mopidy.ext': [ - 'core = mopidy.core:Extension', 'http = mopidy.http:Extension', 'local = mopidy.local:Extension', 'file = mopidy.file:Extension',