From ee57eb58a32e54b337374ac4499ddcae6e0fde6f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 00:07:31 +0200 Subject: [PATCH] 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)