From bc41d55a88f420ebd3b3b4ce284ac773b33f5a8d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 14 Apr 2013 15:35:00 +0200 Subject: [PATCH 01/51] config: Read configs in binary mode --- mopidy/config/__init__.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 97a073f9..98ab9055 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals -import codecs import ConfigParser as configparser import io import logging @@ -57,22 +56,21 @@ def _load(files, defaults, overrides): files = [path.expand_path(f) for f in files] sources = ['builtin-defaults'] + files + ['command-line'] logger.info('Loading config from: %s', ', '.join(sources)) - for default in defaults: # TODO: remove decoding - parser.readfp(io.StringIO(default.decode('utf-8'))) + + # TODO: simply return path to config file for defaults so we can load it + # all in the same way? + for default in defaults: + parser.readfp(io.BytesIO(default)) # Load config from a series of config files for filename in files: - # TODO: if this is the initial load of logging config we might not have - # a logger at this point, we might want to handle this better. try: - with codecs.open(filename, encoding='utf-8') as filehandle: + with io.open(filename, 'rb') as filehandle: parser.readfp(filehandle) except IOError: + # TODO: if this is the initial load of logging config we might not + # have a logger at this point, we might want to handle this better. logger.debug('Config file %s not found; skipping', filename) - continue - except UnicodeDecodeError: - logger.error('Config file %s is not UTF-8 encoded', filename) - sys.exit(1) raw_config = {} for section in parser.sections(): From 6b89051b5e86b9d1978fc543514e3685bc4350ac Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 14 Apr 2013 16:14:41 +0200 Subject: [PATCH 02/51] config: Add encoding support to strings --- mopidy/config/types.py | 2 ++ tests/config/types_test.py | 41 ++++++++++++++++++++++++++++---------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/mopidy/config/types.py b/mopidy/config/types.py index 35ec0a44..1f5c0946 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -80,6 +80,8 @@ class String(ConfigValue): Supported kwargs: ``optional``, ``choices``, and ``secret``. """ def deserialize(self, value): + if not isinstance(value, unicode): + value = value.decode('utf-8') value = value.strip() validators.validate_required(value, not self.optional) validators.validate_choice(value, self.choices) diff --git a/tests/config/types_test.py b/tests/config/types_test.py index 448283b1..9c12b951 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -1,3 +1,5 @@ +# encoding: utf-8 + from __future__ import unicode_literals import logging @@ -30,12 +32,12 @@ class ConfigValueTest(unittest.TestCase): def test_deserialize_passes_through(self): value = types.ConfigValue() - obj = object() - self.assertEqual(obj, value.deserialize(obj)) + sentinel = object() + self.assertEqual(sentinel, value.deserialize(sentinel)) def test_serialize_conversion_to_string(self): value = types.ConfigValue() - self.assertIsInstance(value.serialize(object()), basestring) + self.assertIsInstance(value.serialize(object()), bytes) def test_format_uses_serialize(self): value = types.ConfigValue() @@ -50,22 +52,41 @@ class ConfigValueTest(unittest.TestCase): class StringTest(unittest.TestCase): def test_deserialize_conversion_success(self): value = types.String() - self.assertEqual('foo', value.deserialize(' foo ')) + self.assertEqual('foo', value.deserialize(b' foo ')) + self.assertIsInstance(value.deserialize(b'foo'), unicode) + + def test_deserialize_decodes_utf8(self): + value = types.String() + result = value.deserialize('æøå'.encode('utf-8')) + self.assertEqual('æøå', result) + + def test_deserialize_does_not_double_encode_unicode(self): + value = types.String() + result = value.deserialize('æøå') + self.assertEqual('æøå', result) + + # TODO: add test_deserialize_decodes_string_escapes def test_deserialize_enforces_choices(self): value = types.String(choices=['foo', 'bar', 'baz']) - self.assertEqual('foo', value.deserialize('foo')) - self.assertRaises(ValueError, value.deserialize, 'foobar') + self.assertEqual('foo', value.deserialize(b'foo')) + self.assertRaises(ValueError, value.deserialize, b'foobar') def test_deserialize_enforces_required(self): value = types.String() - self.assertRaises(ValueError, value.deserialize, '') - self.assertRaises(ValueError, value.deserialize, ' ') + self.assertRaises(ValueError, value.deserialize, b'') + self.assertRaises(ValueError, value.deserialize, b' ') def test_deserialize_respects_optional(self): value = types.String(optional=True) - self.assertIsNone(value.deserialize('')) - self.assertIsNone(value.deserialize(' ')) + self.assertIsNone(value.deserialize(b'')) + self.assertIsNone(value.deserialize(b' ')) + + def test_deserialize_decode_failure(self): + value = types.String() + incorrectly_encoded_bytes = u'æøå'.encode('iso-8859-1') + self.assertRaises( + ValueError, value.deserialize, incorrectly_encoded_bytes) def test_serialize_string_escapes(self): value = types.String() From 9f18d50ab04d1c67beeca72be9b6b3afaefbb35a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 14 Apr 2013 17:16:17 +0200 Subject: [PATCH 03/51] config: Fix escapes in string handling --- mopidy/config/types.py | 9 +++++++-- tests/config/types_test.py | 23 ++++++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/mopidy/config/types.py b/mopidy/config/types.py index 1f5c0946..1f57f62c 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -81,7 +81,8 @@ class String(ConfigValue): """ def deserialize(self, value): if not isinstance(value, unicode): - value = value.decode('utf-8') + # TODO: only unescape \n \t and \\? + value = value.decode('string-escape').decode('utf-8') value = value.strip() validators.validate_required(value, not self.optional) validators.validate_choice(value, self.choices) @@ -90,7 +91,11 @@ class String(ConfigValue): return value def serialize(self, value): - return value.encode('utf-8').encode('string-escape') + if isinstance(value, unicode): + for char in ('\\', '\n', '\t'): # TODO: more escapes? + value = value.replace(char, char.encode('unicode-escape')) + value = value.encode('utf-8') + return value class Integer(ConfigValue): diff --git a/tests/config/types_test.py b/tests/config/types_test.py index 9c12b951..d78db461 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -65,7 +65,10 @@ class StringTest(unittest.TestCase): result = value.deserialize('æøå') self.assertEqual('æøå', result) - # TODO: add test_deserialize_decodes_string_escapes + def test_deserialize_handles_escapes(self): + value = types.String(optional=True) + result = value.deserialize(b'a\\t\\nb') + self.assertEqual('a\t\nb', result) def test_deserialize_enforces_choices(self): value = types.String(choices=['foo', 'bar', 'baz']) @@ -88,9 +91,23 @@ class StringTest(unittest.TestCase): self.assertRaises( ValueError, value.deserialize, incorrectly_encoded_bytes) - def test_serialize_string_escapes(self): + def test_serialize_encodes_utf8(self): value = types.String() - self.assertEqual(r'\r\n\t', value.serialize('\r\n\t')) + result = value.serialize('æøå') + self.assertIsInstance(result, bytes) + self.assertEqual('æøå'.encode('utf-8'), result) + + def test_serialize_does_not_encode_bytes(self): + value = types.String() + result = value.serialize('æøå'.encode('utf-8')) + self.assertIsInstance(result, bytes) + self.assertEqual('æøå'.encode('utf-8'), result) + + def test_serialize_handles_escapes(self): + value = types.String() + result = value.serialize('a\n\tb') + self.assertIsInstance(result, bytes) + self.assertEqual(r'a\n\tb'.encode('utf-8'), result) def test_format_masks_secrets(self): value = types.String(secret=True) From 7ed9b8adab24a499353cb36f8459fa2afe6ee632 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 14 Apr 2013 17:34:54 +0200 Subject: [PATCH 04/51] config: Extract encode and decode helpers from String --- mopidy/__main__.py | 4 ++++ mopidy/config/types.py | 26 +++++++++++++++++--------- tests/config/types_test.py | 2 ++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 8f98de5c..163f67ae 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -44,8 +44,12 @@ def main(): extensions = [] # Make sure it is defined before the finally block + # TODO: figure out a way to make the boilerplate in this file reusable in + # scanner and other places we need it. + try: create_file_structures() + # TODO: run raw logging config trough escape code etc, or just validate? logging_config = config_lib.load(config_files, config_overrides) log.setup_logging( logging_config, options.verbosity_level, options.save_debug_log) diff --git a/mopidy/config/types.py b/mopidy/config/types.py index 1f57f62c..3fed89b2 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -8,6 +8,21 @@ from mopidy.utils import path from mopidy.config import validators +def decode(value): + if isinstance(value, unicode): + return value + # TODO: only unescape \n \t and \\? + return value.decode('string-escape').decode('utf-8') + + +def encode(value): + if not isinstance(value, unicode): + return value + for char in ('\\', '\n', '\t'): # TODO: more escapes? + value = value.replace(char, char.encode('unicode-escape')) + return value.encode('utf-8') + + class ConfigValue(object): """Represents a config key's value and how to handle it. @@ -80,10 +95,7 @@ class String(ConfigValue): Supported kwargs: ``optional``, ``choices``, and ``secret``. """ def deserialize(self, value): - if not isinstance(value, unicode): - # TODO: only unescape \n \t and \\? - value = value.decode('string-escape').decode('utf-8') - value = value.strip() + value = decode(value).strip() validators.validate_required(value, not self.optional) validators.validate_choice(value, self.choices) if not value: @@ -91,11 +103,7 @@ class String(ConfigValue): return value def serialize(self, value): - if isinstance(value, unicode): - for char in ('\\', '\n', '\t'): # TODO: more escapes? - value = value.replace(char, char.encode('unicode-escape')) - value = value.encode('utf-8') - return value + return encode(value) class Integer(ConfigValue): diff --git a/tests/config/types_test.py b/tests/config/types_test.py index d78db461..fb23051c 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -10,6 +10,8 @@ from mopidy.config import types from tests import unittest +# TODO: DecodeTest and EncodeTest + class ConfigValueTest(unittest.TestCase): def test_init(self): From d5b8f2ab02715fde32deb1b36bc253d424f90831 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 14 Apr 2013 17:50:16 +0200 Subject: [PATCH 05/51] config: Make List use proper encode/decode helpers --- mopidy/config/types.py | 11 ++++++----- tests/config/types_test.py | 40 ++++++++++++++++++++++++++++++++------ 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/mopidy/config/types.py b/mopidy/config/types.py index 3fed89b2..8a4a4a52 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -157,14 +157,15 @@ class List(ConfigValue): """ def deserialize(self, value): validators.validate_required(value, not self.optional) - if '\n' in value: - values = re.split(r'\s*\n\s*', value.strip()) + if b'\n' in value: + values = re.split(r'\s*\n\s*', value) else: - values = re.split(r'\s*,\s*', value.strip()) - return tuple([v for v in values if v]) + values = re.split(r'\s*,\s*', value) + values = (decode(v).strip() for v in values) + return tuple(v for v in values if v) def serialize(self, value): - return '\n ' + '\n '.join(v.encode('utf-8') for v in value) + return b'\n ' + b'\n '.join(encode(v) for v in value if v) class LogLevel(ConfigValue): diff --git a/tests/config/types_test.py b/tests/config/types_test.py index fb23051c..ddfc06a0 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -179,28 +179,56 @@ class BooleanTest(unittest.TestCase): class ListTest(unittest.TestCase): + # TODO: add test_deserialize_ignores_blank + # TODO: add test_serialize_ignores_blank + # TODO: add test_deserialize_handles_escapes + def test_deserialize_conversion_success(self): value = types.List() expected = ('foo', 'bar', 'baz') - self.assertEqual(expected, value.deserialize('foo, bar ,baz ')) + self.assertEqual(expected, value.deserialize(b'foo, bar ,baz ')) expected = ('foo,bar', 'bar', 'baz') - self.assertEqual(expected, value.deserialize(' foo,bar\nbar\nbaz')) + self.assertEqual(expected, value.deserialize(b' foo,bar\nbar\nbaz')) + + def test_deserialize_creates_tuples(self): + value = types.List(optional=True) + self.assertIsInstance(value.deserialize(b'foo,bar,baz'), tuple) + self.assertIsInstance(value.deserialize(b''), tuple) + + def test_deserialize_decodes_utf8(self): + value = types.List() + + result = value.deserialize('æ, ø, å'.encode('utf-8')) + self.assertEqual(('æ', 'ø', 'å'), result) + + result = value.deserialize('æ\nø\nå'.encode('utf-8')) + self.assertEqual(('æ', 'ø', 'å'), result) + + def test_deserialize_does_not_double_encode_unicode(self): + value = types.List() + + result = value.deserialize('æ, ø, å') + self.assertEqual(('æ', 'ø', 'å'), result) + + result = value.deserialize('æ\nø\nå') + self.assertEqual(('æ', 'ø', 'å'), result) def test_deserialize_enforces_required(self): value = types.List() - self.assertRaises(ValueError, value.deserialize, '') - self.assertRaises(ValueError, value.deserialize, ' ') + self.assertRaises(ValueError, value.deserialize, b'') + self.assertRaises(ValueError, value.deserialize, b' ') def test_deserialize_respects_optional(self): value = types.List(optional=True) - self.assertEqual(tuple(), value.deserialize('')) - self.assertEqual(tuple(), value.deserialize(' ')) + self.assertEqual(tuple(), value.deserialize(b'')) + self.assertEqual(tuple(), value.deserialize(b' ')) def test_serialize(self): value = types.List() result = value.serialize(('foo', 'bar', 'baz')) + self.assertIsInstance(result, bytes) self.assertRegexpMatches(result, r'foo\n\s*bar\n\s*baz') From 805733a2aa601f93b5b57a01cacaad0a1d1b3bbd Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 14 Apr 2013 23:52:39 +0200 Subject: [PATCH 06/51] config: Make tests discoverable and fix broken ones --- mopidy/config/__init__.py | 2 +- tests/config/__init__.py | 0 tests/config/config_test.py | 6 +++--- tests/config/schemas_test.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) create mode 100644 tests/config/__init__.py diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 98ab9055..0267aff5 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -107,7 +107,7 @@ def _validate(raw_config, schemas): items = raw_config[schema.name].items() config[schema.name] = schema.convert(items) except KeyError: - errors.append('%s: section not found.' % name) + errors.append('%s: section not found.' % schema.name) except exceptions.ConfigError as error: for key in error: errors.append('%s/%s: %s' % (schema.name, key, error[key])) diff --git a/tests/config/__init__.py b/tests/config/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/config/config_test.py b/tests/config/config_test.py index dc1b1c59..c90486c7 100644 --- a/tests/config/config_test.py +++ b/tests/config/config_test.py @@ -12,14 +12,14 @@ class LoadConfigTest(unittest.TestCase): self.assertEqual({}, config._load([], [], [])) def test_load_single_default(self): - default = '[foo]\nbar = baz' + default = b'[foo]\nbar = baz' expected = {'foo': {'bar': 'baz'}} result = config._load([], [default], []) self.assertEqual(expected, result) def test_load_defaults(self): - default1 = '[foo]\nbar = baz' - default2 = '[foo2]\n' + default1 = b'[foo]\nbar = baz' + default2 = b'[foo2]\n' expected = {'foo': {'bar': 'baz'}, 'foo2': {}} result = config._load([], [default1, default2], []) self.assertEqual(expected, result) diff --git a/tests/config/schemas_test.py b/tests/config/schemas_test.py index 487136c2..6274f66c 100644 --- a/tests/config/schemas_test.py +++ b/tests/config/schemas_test.py @@ -94,7 +94,7 @@ class ExtensionConfigSchemaTest(unittest.TestCase): class LogLevelConfigSchemaTest(unittest.TestCase): def test_conversion(self): - schema = schemas.LogLevelConfigSchema() + schema = schemas.LogLevelConfigSchema('test') result = schema.convert([('foo.bar', 'DEBUG'), ('baz', 'INFO')]) self.assertEqual(logging.DEBUG, result['foo.bar']) From 4826dc7cace647764c0123407f6d1787ade53141 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 00:07:07 +0200 Subject: [PATCH 07/51] config: Support lists in required validator --- mopidy/config/validators.py | 2 +- tests/config/validator_tests.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/mopidy/config/validators.py b/mopidy/config/validators.py index 9e374ce5..a0ca25d9 100644 --- a/mopidy/config/validators.py +++ b/mopidy/config/validators.py @@ -9,7 +9,7 @@ def validate_required(value, required): Normally called in :meth:`~mopidy.config.types.ConfigValue.deserialize` on the raw string, _not_ the converted value. """ - if required and not value.strip(): + if required and not value: raise ValueError('must be set.') diff --git a/tests/config/validator_tests.py b/tests/config/validator_tests.py index 57489b6b..63ef8ca6 100644 --- a/tests/config/validator_tests.py +++ b/tests/config/validator_tests.py @@ -57,11 +57,13 @@ class ValidateRequiredTest(unittest.TestCase): validators.validate_required('foo', False) validators.validate_required('', False) validators.validate_required(' ', False) + validators.validate_required([], False) def test_passes_when_required_and_set(self): validators.validate_required('foo', True) validators.validate_required(' foo ', True) + validators.validate_required([1], True) def test_blocks_when_required_and_emtpy(self): self.assertRaises(ValueError, validators.validate_required, '', True) - self.assertRaises(ValueError, validators.validate_required, ' ', True) + self.assertRaises(ValueError, validators.validate_required, [], True) From ee57eb58a32e54b337374ac4499ddcae6e0fde6f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 00:07:31 +0200 Subject: [PATCH 08/51] config: Strict config value init kwargs, also adds Secret --- mopidy/config/__init__.py | 2 +- mopidy/config/types.py | 153 +++++++++++++++++-------------------- tests/config/types_test.py | 57 +++++--------- 3 files changed, 90 insertions(+), 122 deletions(-) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 0267aff5..73b384c2 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -27,7 +27,7 @@ _audio_schema['output'] = String() _proxy_schema = ConfigSchema('proxy') _proxy_schema['hostname'] = Hostname(optional=True) _proxy_schema['username'] = String(optional=True) -_proxy_schema['password'] = String(optional=True, secret=True) +_proxy_schema['password'] = Secret(optional=True) # NOTE: if multiple outputs ever comes something like LogLevelConfigSchema #_outputs_schema = config.AudioOutputConfigSchema() diff --git a/mopidy/config/types.py b/mopidy/config/types.py index 8a4a4a52..6543b62e 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -23,6 +23,15 @@ def encode(value): return value.encode('utf-8') +class ExpandedPath(bytes): + def __new__(self, value): + expanded = path.expand_path(value) + return super(ExpandedPath, self).__new__(self, expanded) + + def __init__(self, value): + self.original = value + + class ConfigValue(object): """Represents a config key's value and how to handle it. @@ -40,64 +49,32 @@ class ConfigValue(object): the code interacting with the config should simply skip None config values. """ - choices = None - """ - Collection of valid choices for converted value. Must be combined with - :func:`~mopidy.config.validators.validate_choice` in :meth:`deserialize` - do any thing. - """ - - minimum = None - """ - Minimum of converted value. Must be combined with - :func:`~mopidy.config.validators.validate_minimum` in :meth:`deserialize` - do any thing. - """ - - maximum = None - """ - Maximum of converted value. Must be combined with - :func:`~mopidy.config.validators.validate_maximum` in :meth:`deserialize` - do any thing. - """ - - optional = None - """Indicate if this field is required.""" - - secret = None - """Indicate if we should mask the when printing for human consumption.""" - - def __init__(self, **kwargs): - self.choices = kwargs.get('choices') - self.minimum = kwargs.get('minimum') - self.maximum = kwargs.get('maximum') - self.optional = kwargs.get('optional') - self.secret = kwargs.get('secret') - def deserialize(self, value): """Cast raw string to appropriate type.""" return value def serialize(self, value): """Convert value back to string for saving.""" - return str(value) + return bytes(value) def format(self, value): """Format value for display.""" - if self.secret and value is not None: - return '********' return self.serialize(value) class String(ConfigValue): - """String value + """String value. - Supported kwargs: ``optional``, ``choices``, and ``secret``. + Is decoded as utf-8 and \\n \\t escapes should work and be preserved. """ + def __init__(self, optional=False, choices=None): + self._required = not optional + self._choices = choices + def deserialize(self, value): value = decode(value).strip() - validators.validate_required(value, not self.optional) - validators.validate_choice(value, self.choices) + validators.validate_required(value, self._required) + validators.validate_choice(value, self._choices) if not value: return None return value @@ -106,29 +83,46 @@ class String(ConfigValue): return encode(value) -class Integer(ConfigValue): - """Integer value +class Secret(ConfigValue): + """String value. - Supported kwargs: ``choices``, ``minimum``, ``maximum``, and ``secret`` + Masked when being displayed, and is not decoded. """ + def __init__(self, optional=False, choices=None): + self._required = not optional + + def deserialize(self, value): + validators.validate_required(value, self._required) + return value + + def format(self, value): + return '********' + + +class Integer(ConfigValue): + """Integer value.""" + + def __init__(self, minimum=None, maximum=None, choices=None): + self._minimum = minimum + self._maximum = maximum + self._choices = choices + def deserialize(self, value): value = int(value) - validators.validate_choice(value, self.choices) - validators.validate_minimum(value, self.minimum) - validators.validate_maximum(value, self.maximum) + validators.validate_choice(value, self._choices) + validators.validate_minimum(value, self._minimum) + validators.validate_maximum(value, self._maximum) return value class Boolean(ConfigValue): - """Boolean value + """Boolean value. Accepts ``1``, ``yes``, ``true``, and ``on`` with any casing as :class:`True`. Accepts ``0``, ``no``, ``false``, and ``off`` with any casing as :class:`False`. - - Supported kwargs: ``secret`` """ true_values = ('1', 'yes', 'true', 'on') false_values = ('0', 'no', 'false', 'off') @@ -138,7 +132,6 @@ class Boolean(ConfigValue): return True elif value.lower() in self.false_values: return False - raise ValueError('invalid value for boolean: %r' % value) def serialize(self, value): @@ -149,32 +142,33 @@ class Boolean(ConfigValue): class List(ConfigValue): - """List value + """List value. - Supports elements split by commas or newlines. - - Supported kwargs: ``optional`` and ``secret`` + Supports elements split by commas or newlines. Newlines take presedence and + empty list items will be filtered out. """ + def __init__(self, optional=False): + self._required = not optional + def deserialize(self, value): - validators.validate_required(value, not self.optional) if b'\n' in value: values = re.split(r'\s*\n\s*', value) else: values = re.split(r'\s*,\s*', value) values = (decode(v).strip() for v in values) - return tuple(v for v in values if v) + values = filter(None, values) + validators.validate_required(values, self._required) + return tuple(values) def serialize(self, value): return b'\n ' + b'\n '.join(encode(v) for v in value if v) class LogLevel(ConfigValue): - """Log level value + """Log level value. Expects one of ``critical``, ``error``, ``warning``, ``info``, ``debug`` with any casing. - - Supported kwargs: ``secret`` """ levels = { 'critical': logging.CRITICAL, @@ -193,12 +187,13 @@ class LogLevel(ConfigValue): class Hostname(ConfigValue): - """Hostname value + """Network hostname value.""" + + def __init__(self, optional=False): + self._required = not optional - Supported kwargs: ``optional`` and ``secret`` - """ def deserialize(self, value): - validators.validate_required(value, not self.optional) + validators.validate_required(value, self._required) if not value.strip(): return None try: @@ -209,26 +204,14 @@ class Hostname(ConfigValue): class Port(Integer): - """Port value + """Network port value. - Expects integer in the range 1-65535 - - Supported kwargs: ``choices`` and ``secret`` + Expects integer in the range 0-65535, zero tells the kernel to simply + allocate a port for us. """ # TODO: consider probing if port is free or not? - def __init__(self, **kwargs): - super(Port, self).__init__(**kwargs) - self.minimum = 1 - self.maximum = 2 ** 16 - 1 - - -class ExpandedPath(bytes): - def __new__(self, value): - expanded = path.expand_path(value) - return super(ExpandedPath, self).__new__(self, expanded) - - def __init__(self, value): - self.original = value + def __init__(self, choices=None): + super(Port, self).__init__(minimum=0, maximum=2**16-1, choices=choices) class Path(ConfigValue): @@ -248,10 +231,14 @@ class Path(ConfigValue): Supported kwargs: ``optional``, ``choices``, and ``secret`` """ + def __init__(self, optional=False, choices=None): + self._required = not optional + self._choices = choices + def deserialize(self, value): value = value.strip() - validators.validate_required(value, not self.optional) - validators.validate_choice(value, self.choices) + validators.validate_required(value, self._required) + validators.validate_choice(value, self._choices) if not value: return None return ExpandedPath(value) diff --git a/tests/config/types_test.py b/tests/config/types_test.py index ddfc06a0..fe616b77 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -14,24 +14,6 @@ from tests import unittest class ConfigValueTest(unittest.TestCase): - def test_init(self): - value = types.ConfigValue() - self.assertIsNone(value.choices) - self.assertIsNone(value.maximum) - self.assertIsNone(value.minimum) - self.assertIsNone(value.optional) - self.assertIsNone(value.secret) - - def test_init_with_params(self): - kwargs = {'choices': ['foo'], 'minimum': 0, 'maximum': 10, - 'secret': True, 'optional': True} - value = types.ConfigValue(**kwargs) - self.assertEqual(['foo'], value.choices) - self.assertEqual(0, value.minimum) - self.assertEqual(10, value.maximum) - self.assertEqual(True, value.optional) - self.assertEqual(True, value.secret) - def test_deserialize_passes_through(self): value = types.ConfigValue() sentinel = object() @@ -46,10 +28,6 @@ class ConfigValueTest(unittest.TestCase): obj = object() self.assertEqual(value.serialize(obj), value.format(obj)) - def test_format_masks_secrets(self): - value = types.ConfigValue(secret=True) - self.assertEqual('********', value.format(object())) - class StringTest(unittest.TestCase): def test_deserialize_conversion_success(self): @@ -80,7 +58,6 @@ class StringTest(unittest.TestCase): def test_deserialize_enforces_required(self): value = types.String() self.assertRaises(ValueError, value.deserialize, b'') - self.assertRaises(ValueError, value.deserialize, b' ') def test_deserialize_respects_optional(self): value = types.String(optional=True) @@ -111,8 +88,24 @@ class StringTest(unittest.TestCase): self.assertIsInstance(result, bytes) self.assertEqual(r'a\n\tb'.encode('utf-8'), result) - def test_format_masks_secrets(self): - value = types.String(secret=True) + +class SecretTest(unittest.TestCase): + def test_deserialize_passes_through(self): + value = types.Secret() + result = value.deserialize(b'foo') + self.assertIsInstance(result, bytes) + self.assertEqual(b'foo', result) + + def test_deserialize_enforces_required(self): + value = types.Secret() + self.assertRaises(ValueError, value.deserialize, b'') + + def test_serialize_conversion_to_string(self): + value = types.Secret() + self.assertIsInstance(value.serialize(object()), bytes) + + def test_format_masks_value(self): + value = types.Secret() self.assertEqual('********', value.format('s3cret')) @@ -145,10 +138,6 @@ class IntegerTest(unittest.TestCase): self.assertEqual(5, value.deserialize('5')) self.assertRaises(ValueError, value.deserialize, '15') - def test_format_masks_secrets(self): - value = types.Integer(secret=True) - self.assertEqual('********', value.format('1337')) - class BooleanTest(unittest.TestCase): def test_deserialize_conversion_success(self): @@ -173,10 +162,6 @@ class BooleanTest(unittest.TestCase): self.assertEqual('true', value.serialize(True)) self.assertEqual('false', value.serialize(False)) - def test_format_masks_secrets(self): - value = types.Boolean(secret=True) - self.assertEqual('********', value.format('true')) - class ListTest(unittest.TestCase): # TODO: add test_deserialize_ignores_blank @@ -218,12 +203,10 @@ class ListTest(unittest.TestCase): def test_deserialize_enforces_required(self): value = types.List() self.assertRaises(ValueError, value.deserialize, b'') - self.assertRaises(ValueError, value.deserialize, b' ') def test_deserialize_respects_optional(self): value = types.List(optional=True) self.assertEqual(tuple(), value.deserialize(b'')) - self.assertEqual(tuple(), value.deserialize(b' ')) def test_serialize(self): value = types.List() @@ -277,7 +260,6 @@ class HostnameTest(unittest.TestCase): def test_deserialize_enforces_required(self, getaddrinfo_mock): value = types.Hostname() self.assertRaises(ValueError, value.deserialize, '') - self.assertRaises(ValueError, value.deserialize, ' ') self.assertEqual(0, getaddrinfo_mock.call_count) @mock.patch('socket.getaddrinfo') @@ -291,6 +273,7 @@ class HostnameTest(unittest.TestCase): class PortTest(unittest.TestCase): def test_valid_ports(self): value = types.Port() + self.assertEqual(0, value.deserialize('0')) self.assertEqual(1, value.deserialize('1')) self.assertEqual(80, value.deserialize('80')) self.assertEqual(6600, value.deserialize('6600')) @@ -300,7 +283,6 @@ class PortTest(unittest.TestCase): value = types.Port() self.assertRaises(ValueError, value.deserialize, '65536') self.assertRaises(ValueError, value.deserialize, '100000') - self.assertRaises(ValueError, value.deserialize, '0') self.assertRaises(ValueError, value.deserialize, '-1') self.assertRaises(ValueError, value.deserialize, '') @@ -334,7 +316,6 @@ class PathTest(unittest.TestCase): def test_deserialize_enforces_required(self): value = types.Path() self.assertRaises(ValueError, value.deserialize, '') - self.assertRaises(ValueError, value.deserialize, ' ') def test_deserialize_respects_optional(self): value = types.Path(optional=True) From ad25a60ba53e6b1540f2cbb5b232e660e3a789b7 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 00:08:23 +0200 Subject: [PATCH 09/51] config: Update extensions with respect to config changes --- mopidy/backends/spotify/__init__.py | 2 +- mopidy/frontends/mpd/__init__.py | 2 +- mopidy/frontends/scrobbler/__init__.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/backends/spotify/__init__.py b/mopidy/backends/spotify/__init__.py index 55d0e3d7..3cee609a 100644 --- a/mopidy/backends/spotify/__init__.py +++ b/mopidy/backends/spotify/__init__.py @@ -19,7 +19,7 @@ class Extension(ext.Extension): def get_config_schema(self): schema = super(Extension, self).get_config_schema() schema['username'] = config.String() - schema['password'] = config.String(secret=True) + schema['password'] = config.Secret() schema['bitrate'] = config.Integer(choices=(96, 160, 320)) schema['timeout'] = config.Integer(minimum=0) schema['cache_dir'] = config.Path() diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 04c00c2b..276be450 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -20,7 +20,7 @@ class Extension(ext.Extension): schema = super(Extension, self).get_config_schema() schema['hostname'] = config.Hostname() schema['port'] = config.Port() - schema['password'] = config.String(optional=True, secret=True) + schema['password'] = config.Secret(optional=True) schema['max_connections'] = config.Integer(minimum=1) schema['connection_timeout'] = config.Integer(minimum=1) return schema diff --git a/mopidy/frontends/scrobbler/__init__.py b/mopidy/frontends/scrobbler/__init__.py index dcc6f195..c08bc15e 100644 --- a/mopidy/frontends/scrobbler/__init__.py +++ b/mopidy/frontends/scrobbler/__init__.py @@ -19,7 +19,7 @@ class Extension(ext.Extension): def get_config_schema(self): schema = super(Extension, self).get_config_schema() schema['username'] = config.String() - schema['password'] = config.String(secret=True) + schema['password'] = config.Secret() return schema def validate_environment(self): From 0ede12f0506b197dbe0993faed8deb2b5788f98c Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 00:08:48 +0200 Subject: [PATCH 10/51] ext: Update and fix extensiondev docs with respect to config --- docs/extensiondev.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/extensiondev.rst b/docs/extensiondev.rst index d705cd60..106ae219 100644 --- a/docs/extensiondev.rst +++ b/docs/extensiondev.rst @@ -230,12 +230,12 @@ and ``password``. version = __version__ def get_default_config(self): - return default_config + return bytes(default_config) def get_config_schema(self): schema = super(Extension, self).get_config_schema() - schema['username'] = config.String(required=True) - schema['password'] = config.String(required=True, secret=True) + schema['username'] = config.String() + schema['password'] = config.Secret() return schema def validate_environment(self): From a7035063c28f8606ca3403b4cb2c373114b4f918 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 15 Apr 2013 11:58:54 +0200 Subject: [PATCH 11/51] docs: Note on debugging GStreamer --- docs/troubleshooting.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/troubleshooting.rst b/docs/troubleshooting.rst index 4a634a99..e590c86c 100644 --- a/docs/troubleshooting.rst +++ b/docs/troubleshooting.rst @@ -56,3 +56,21 @@ system is deadlocking. If you have the ``pkill`` command installed, you can use this by simply running:: pkill -SIGUSR1 mopidy + + +Debugging GStreamer +=================== + +If you really want to dig in and debug GStreamer behaviour, then check out the +`Debugging section +`_ +of GStreamer's documentation for your options. Note that Mopidy does not +support the GStreamer command line options, like ``--gst-debug-level=3``, but +setting GStreamer environment variables, like :envvar:`GST_DEBUG`, works with +Mopidy. For example, to run Mopidy with debug logging and GStreamer logging at +level 3, you can run:: + + GST_DEBUG=3 mopidy -v + +This will produce a lot of output, but given some GStreamer knowledge this is +very useful for debugging GStreamer pipeline issues. From fb0810bf9edff591341972d20edebbc141196f8a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 17:43:14 +0200 Subject: [PATCH 12/51] config: Specify we want bytes for default configs --- mopidy/ext.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/ext.py b/mopidy/ext.py index 13df48c2..9cea1bd5 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -34,9 +34,9 @@ class Extension(object): """ def get_default_config(self): - """The extension's default config as a string + """The extension's default config as a bytestring - :returns: string + :returns: bytes """ raise NotImplementedError( 'Add at least a config section with "enabled = true"') From 90067a2128d15c6803479d219567e81912c73d6d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 15 Apr 2013 18:28:12 +0200 Subject: [PATCH 13/51] docs: Mark command line options with :option: --- docs/config.rst | 8 ++++---- docs/devtools.rst | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/config.rst b/docs/config.rst index 3b31f0dd..44e65875 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -28,9 +28,9 @@ the config values you want to change. If you want to keep the default for a config value, you **should not** add it to ``~/.config/mopidy/mopidy.conf``. To see what's the effective configuration for your Mopidy installation, you can -run ``mopidy --show-config``. It will print your full effective config with -passwords masked out so that you safely can share the output with others for -debugging. +run :option:`mopidy --show-config`. It will print your full effective config +with passwords masked out so that you safely can share the output with others +for debugging. You can find a description of all config values belonging to Mopidy's core below, together with their default values. In addition, all :ref:`extensions @@ -95,7 +95,7 @@ Core configuration values .. confval:: logging/debug_file The file to dump debug log data to when Mopidy is run with the - :option:`--save-debug-log` option. + :option:`mopidy --save-debug-log` option. .. confval:: logging.levels/* diff --git a/docs/devtools.rst b/docs/devtools.rst index 7b5b2f81..bc066cd0 100644 --- a/docs/devtools.rst +++ b/docs/devtools.rst @@ -40,7 +40,8 @@ sends all requests to both, returning the primary response to the client and then printing any diff in the two responses. Note that this tool depends on ``gevent`` unlike the rest of Mopidy at the time -of writing. See ``--help`` for available options. Sample session:: +of writing. See :option:`tools/debug-proxy.py --help` for available options. +Sample session:: [127.0.0.1]:59714 listallinfo From ee915fbf7a1ecdbbb49cb193d331e8df3bcdd62c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 15 Apr 2013 18:51:58 +0200 Subject: [PATCH 14/51] docs: Add command line options --- docs/running.rst | 59 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/docs/running.rst b/docs/running.rst index b81dbef7..58f7d591 100644 --- a/docs/running.rst +++ b/docs/running.rst @@ -25,7 +25,48 @@ mopidy command .. program:: mopidy -TODO: Document all command line options +.. cmdoption:: --version + + Show Mopidy's version number and exit. + +.. cmdoption:: -h, --help + + Show help message and exit. + +.. cmdoption:: -q, --quite + + Show less output: warning level and higher. + +.. cmdoption:: -v, --verbose + + Show more output: debug level and higher. + +.. cmdoption:: --save-debug-log + + Save debug log to the file specified in the :confval:`logging/debug_file` + config value, typically ``./mopidy.conf``. + +.. cmdoption:: --show-config + + Show the current effective config. All configuration sources are merged + together to show the effective document. Secret values like passwords are + masked out. Config for disabled extensions are not included. + +.. cmdoption:: --list-deps + + List dependencies and their versions. + +.. cmdoption:: --config + + Specify config file to use. To use multiple config files, separate them + with colon. The later files override the earlier ones if there's a + conflict. + +.. cmdoption:: -o