From 5b60e9695e1785026f3ecd96eefe86b9b1c03191 Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Mon, 28 Jan 2019 23:07:39 -0800 Subject: [PATCH 01/16] Add better handling of deprecated configs --- homeassistant/components/freedns/__init__.py | 33 +++-- homeassistant/helpers/config_validation.py | 119 ++++++++++++++++++- tests/components/freedns/test_init.py | 2 +- tests/helpers/test_config_validation.py | 5 + 4 files changed, 141 insertions(+), 18 deletions(-) diff --git a/homeassistant/components/freedns/__init__.py b/homeassistant/components/freedns/__init__.py index ec38bb59cc7822..ad754d6af15550 100644 --- a/homeassistant/components/freedns/__init__.py +++ b/homeassistant/components/freedns/__init__.py @@ -13,7 +13,7 @@ import voluptuous as vol from homeassistant.const import (CONF_URL, CONF_ACCESS_TOKEN, - CONF_UPDATE_INTERVAL) + CONF_UPDATE_INTERVAL, CONF_SCAN_INTERVAL) import homeassistant.helpers.config_validation as cv _LOGGER = logging.getLogger(__name__) @@ -26,21 +26,32 @@ UPDATE_URL = 'https://freedns.afraid.org/dynamic/update.php' CONFIG_SCHEMA = vol.Schema({ - DOMAIN: vol.Schema({ - vol.Exclusive(CONF_URL, DOMAIN): cv.string, - vol.Exclusive(CONF_ACCESS_TOKEN, DOMAIN): cv.string, - vol.Optional(CONF_UPDATE_INTERVAL, default=DEFAULT_INTERVAL): vol.All( - cv.time_period, cv.positive_timedelta), - - }) + DOMAIN: vol.All( + vol.Schema({ + vol.Exclusive(CONF_URL, DOMAIN): cv.string, + vol.Exclusive(CONF_ACCESS_TOKEN, DOMAIN): cv.string, + vol.Optional(CONF_UPDATE_INTERVAL): + vol.All(cv.time_period, cv.positive_timedelta), + vol.Optional(CONF_SCAN_INTERVAL): + vol.All(cv.time_period, cv.positive_timedelta), + }), + cv.deprecated( + CONF_UPDATE_INTERVAL, + replacement_key=CONF_SCAN_INTERVAL, + invalidation_version='1.0.0', + default=DEFAULT_INTERVAL + ), + cv.has_at_most_one_key(CONF_SCAN_INTERVAL, CONF_UPDATE_INTERVAL) + ) }, extra=vol.ALLOW_EXTRA) async def async_setup(hass, config): """Initialize the FreeDNS component.""" - url = config[DOMAIN].get(CONF_URL) - auth_token = config[DOMAIN].get(CONF_ACCESS_TOKEN) - update_interval = config[DOMAIN].get(CONF_UPDATE_INTERVAL) + conf = config[DOMAIN] + url = conf.get(CONF_URL) + auth_token = conf.get(CONF_ACCESS_TOKEN) + update_interval = conf[CONF_SCAN_INTERVAL] session = hass.helpers.aiohttp_client.async_get_clientsession() diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 3a4b9ced0abf60..19961646aca767 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -16,7 +16,8 @@ CONF_ALIAS, CONF_ENTITY_ID, CONF_VALUE_TEMPLATE, WEEKDAYS, CONF_CONDITION, CONF_BELOW, CONF_ABOVE, CONF_TIMEOUT, SUN_EVENT_SUNSET, SUN_EVENT_SUNRISE, CONF_UNIT_SYSTEM_IMPERIAL, CONF_UNIT_SYSTEM_METRIC, - ENTITY_MATCH_ALL, CONF_ENTITY_NAMESPACE) + ENTITY_MATCH_ALL, CONF_ENTITY_NAMESPACE, MAJOR_VERSION, MINOR_VERSION, + PATCH_VERSION) from homeassistant.core import valid_entity_id, split_entity_id from homeassistant.exceptions import TemplateError import homeassistant.util.dt as dt_util @@ -67,6 +68,26 @@ def validate(obj: Dict) -> Dict: return validate +def has_at_most_one_key(*keys: str) -> Callable: + """Validate that only one key exists.""" + def validate(obj: Dict) -> Dict: + if not isinstance(obj, dict): + raise vol.Invalid('expected dictionary') + + count = 0 + for k in obj.keys(): + if k in keys: + count += 1 + + if count > 1: + raise vol.Invalid( + 'must contain at most one of {}.'.format(', '.join(keys)) + ) + return obj + + return validate + + def boolean(value: Any) -> bool: """Validate and coerce a boolean value.""" if isinstance(value, str): @@ -520,19 +541,105 @@ def ensure_list_csv(value: Any) -> Sequence: return ensure_list(value) -def deprecated(key): - """Log key as deprecated.""" +def deprecated(key, replacement_key=None, invalidation_version=None, + default=None): + """Log key as deprecated and provide a replacement (if exists).""" module_name = inspect.getmodule(inspect.stack()[1][0]).__name__ + if replacement_key and invalidation_version: + warning = ("The '{key}' option (with value '{value}') is deprecated," + " please replace it with '{replacement_key}'. This option" + " will become invalid in version {invalidation_version}.") + elif replacement_key: + warning = ("The '{key}' option (with value '{value}') is deprecated," + " please replace it with '{replacement_key}'.") + elif invalidation_version: + warning = ("The '{key}' option (with value '{value}') is deprecated," + " please remove it from your configuration. This option" + " will become invalid in version {invalidation_version}.") + else: + warning = ("The '{key}' option (with value '{value}') is deprecated," + " please remove it from your configuration.") + + def check_for_invalid_version(value): + """Raise error if current version has reach invalidation.""" + if not invalidation_version: + return + + major_version, minor_version, patch_version = \ + map(int, invalidation_version.split('.', 2)) + if (MAJOR_VERSION >= major_version + and MINOR_VERSION >= minor_version + and PATCH_VERSION >= patch_version): + raise vol.Invalid( + warning.format( + key=key, + value=value, + replacement_key=replacement_key, + invalidation_version=invalidation_version + ) + ) + def validator(config): """Check if key is in config and log warning.""" if key in config: - logging.getLogger(module_name).warning( - "The '%s' option (with value '%s') is deprecated, please " - "remove it from your configuration.", key, config[key]) + value = config[key] + check_for_invalid_version(value) + StyleAdapter(logging.getLogger(module_name)).warning( + warning, + key=key, + value=value, + replacement_key=replacement_key, + invalidation_version=invalidation_version + ) + config.pop(key) + else: + value = default + config[replacement_key] = value return config + # Adapted from: https://stackoverflow.com/a/24683360/2267718 + class BraceMessage: + """Represents a logging message with brace style arguments.""" + + def __init__(self, fmt, args, kwargs): + """Initialize a new BraceMessage object.""" + self._fmt = fmt + self._args = args + self._kwargs = kwargs + + def __str__(self): + """Convert the object to a string for logging.""" + return str(self._fmt).format(*self._args, **self._kwargs) + + class StyleAdapter(logging.LoggerAdapter): + """Represents an adapter wrapping the logger allowing BraceMessages.""" + + def __init__(self, logger, extra=None): + """Initialize a new StyleAdapter for the provided logger.""" + super(StyleAdapter, self).__init__(logger, extra or {}) + + def log(self, level, msg, *args, **kwargs): + """Log the message provided at the appropriate level.""" + if self.isEnabledFor(level): + msg, log_kwargs = self.process(msg, kwargs) + self.logger._log( # pylint: disable=protected-access + level, BraceMessage(msg, args, kwargs), (), **log_kwargs + ) + + def process(self, msg, kwargs): + """Process the keyward args in preparation for logging.""" + return ( + msg, + { + key: kwargs[key] + for k in inspect.getfullargspec( + self.logger._log # pylint: disable=protected-access + ).args[1:] if k in kwargs + } + ) + return validator diff --git a/tests/components/freedns/test_init.py b/tests/components/freedns/test_init.py index b8e38e9c3a8f0e..784926912cdfe2 100644 --- a/tests/components/freedns/test_init.py +++ b/tests/components/freedns/test_init.py @@ -40,7 +40,7 @@ def test_setup(hass, aioclient_mock): result = yield from async_setup_component(hass, freedns.DOMAIN, { freedns.DOMAIN: { 'access_token': ACCESS_TOKEN, - 'update_interval': UPDATE_INTERVAL, + 'scan_interval': UPDATE_INTERVAL, } }) assert result diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index 119725b06ddc40..72425beef6f16f 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -511,6 +511,11 @@ def test_deprecated(caplog): "please remove it from your configuration.") in caplog.text +def test_deprecated_with_replacement_invalidation_version(caplog): + """Test deprecation log when a replacement is provided.""" + pass + + def test_key_dependency(): """Test key_dependency validator.""" schema = vol.Schema(cv.key_dependency('beer', 'soda')) From 44649e0a3e1daabae00596718c0e1f7e0d0e456e Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Tue, 29 Jan 2019 08:34:09 -0800 Subject: [PATCH 02/16] Embed the call to has_at_most_one_key in deprecated --- homeassistant/components/freedns/__init__.py | 3 +-- homeassistant/helpers/config_validation.py | 8 +++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/homeassistant/components/freedns/__init__.py b/homeassistant/components/freedns/__init__.py index ad754d6af15550..7da51cd42e4bf3 100644 --- a/homeassistant/components/freedns/__init__.py +++ b/homeassistant/components/freedns/__init__.py @@ -40,8 +40,7 @@ replacement_key=CONF_SCAN_INTERVAL, invalidation_version='1.0.0', default=DEFAULT_INTERVAL - ), - cv.has_at_most_one_key(CONF_SCAN_INTERVAL, CONF_UPDATE_INTERVAL) + ) ) }, extra=vol.ALLOW_EXTRA) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 19961646aca767..52da760632f76c 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -592,11 +592,13 @@ def validator(config): replacement_key=replacement_key, invalidation_version=invalidation_version ) - config.pop(key) + if replacement_key: + config.pop(key) else: value = default - config[replacement_key] = value - + if replacement_key: + config[replacement_key] = value + return has_at_most_one_key(key, replacement_key)(config) return config # Adapted from: https://stackoverflow.com/a/24683360/2267718 From 98c0f41810b849bf3499944a426f8bb26305c8e9 Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Tue, 29 Jan 2019 08:57:53 -0800 Subject: [PATCH 03/16] Add tests for checking the deprecated logs --- homeassistant/helpers/config_validation.py | 3 +- tests/helpers/test_config_validation.py | 66 +++++++++++++++++++--- 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 52da760632f76c..d28da2deceb6e6 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -69,8 +69,9 @@ def validate(obj: Dict) -> Dict: def has_at_most_one_key(*keys: str) -> Callable: - """Validate that only one key exists.""" + """Validate that zero keys exist or one key exists.""" def validate(obj: Dict) -> Dict: + """Test zero keys exist or one key exists in dict.""" if not isinstance(obj, dict): raise vol.Invalid('expected dictionary') diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index 72425beef6f16f..8bdb25c913945a 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -489,19 +489,37 @@ def test_datetime(): schema('2016-11-23T18:59:08') -def test_deprecated(caplog): - """Test deprecation log.""" +def test_deprecated_logging(caplog): + """Test deprecation logs the correct message in each situation.""" schema = vol.Schema({ 'venus': cv.boolean, - 'mars': cv.boolean + 'mars': cv.boolean, + 'jupiter': cv.boolean }) + deprecated_schema = vol.All( cv.deprecated('mars'), schema ) + deprecated_schema_with_invalidation_version = vol.All( + cv.deprecated('mars', invalidation_version='1.0.0'), + schema + ) + + deprecated_schema_with_replacement_key = vol.All( + cv.deprecated('mars', replacement_key='jupiter'), + schema + ) + + deprecated_schema_with_invalidation_version_and_replacement_key = vol.All( + cv.deprecated( + 'mars', replacement_key='jupiter', invalidation_version='1.0.0' + ), + schema + ) + deprecated_schema({'venus': True}) - # pylint: disable=len-as-condition assert len(caplog.records) == 0 deprecated_schema({'mars': True}) @@ -509,11 +527,33 @@ def test_deprecated(caplog): assert caplog.records[0].name == __name__ assert ("The 'mars' option (with value 'True') is deprecated, " "please remove it from your configuration.") in caplog.text + caplog.clear() + assert len(caplog.records) == 0 + deprecated_schema_with_invalidation_version({'mars': True}) + assert len(caplog.records) == 1 + assert ("The 'mars' option (with value 'True') is deprecated, " + "please remove it from your configuration. " + "This option will become invalid in version 1.0.0.") in caplog.text + caplog.clear() + assert len(caplog.records) == 0 -def test_deprecated_with_replacement_invalidation_version(caplog): - """Test deprecation log when a replacement is provided.""" - pass + deprecated_schema_with_replacement_key({'mars': True}) + assert len(caplog.records) == 1 + assert ("The 'mars' option (with value 'True') is deprecated, " + "please replace it with 'jupiter'.") in caplog.text + caplog.clear() + assert len(caplog.records) == 0 + + deprecated_schema_with_invalidation_version_and_replacement_key( + {'mars': True} + ) + assert len(caplog.records) == 1 + assert ("The 'mars' option (with value 'True') is deprecated, " + "please replace it with 'jupiter'. This option will become " + "invalid in version 1.0.0.") in caplog.text + caplog.clear() + assert len(caplog.records) == 0 def test_key_dependency(): @@ -535,6 +575,18 @@ def test_key_dependency(): schema(value) +def test_has_at_most_one_key(): + """Test has_at_most_one_key validator.""" + schema = vol.Schema(cv.has_at_most_one_key('beer', 'soda')) + + for value in (None, [], {'beer': None, 'soda': None}): + with pytest.raises(vol.MultipleInvalid): + schema(value) + + for value in ({}, {'beer': None}, {'soda': None}): + schema(value) + + def test_has_at_least_one_key(): """Test has_at_least_one_key validator.""" schema = vol.Schema(cv.has_at_least_one_key('beer', 'soda')) From 67acc637249f05892a94a3f9014980575b23ea9b Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Tue, 29 Jan 2019 22:47:10 -0800 Subject: [PATCH 04/16] Add thoroughly documented tests --- homeassistant/helpers/config_validation.py | 38 ++- tests/helpers/test_config_validation.py | 270 +++++++++++++++++++-- 2 files changed, 280 insertions(+), 28 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index d28da2deceb6e6..294d974396157c 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -7,7 +7,7 @@ from socket import _GLOBAL_DEFAULT_TIMEOUT import logging import inspect -from typing import Any, Union, TypeVar, Callable, Sequence, Dict +from typing import Any, Union, TypeVar, Callable, Sequence, Dict, Optional import voluptuous as vol @@ -542,9 +542,23 @@ def ensure_list_csv(value: Any) -> Sequence: return ensure_list(value) -def deprecated(key, replacement_key=None, invalidation_version=None, - default=None): - """Log key as deprecated and provide a replacement (if exists).""" +def deprecated(key: str, + replacement_key: Optional[str] = None, + invalidation_version: Optional[str] = None, + default: Optional[Any] = None): + """ + Log key as deprecated and provide a replacement (if exists). + + Expected behavior: + - Outputs the appropriate deprecation warning if key is detected + - Processes schema moving the value from key to replacement_key + - Processes schema changing nothing if only replacement_key provided + - No warning if only replacement_key provided + - No warning if neither key nor replacement_key are provided + - Adds replacement_key with default value in this case + - Once the invalidation_version is crossed, raises vol.Invalid if key + is detected + """ module_name = inspect.getmodule(inspect.stack()[1][0]).__name__ if replacement_key and invalidation_version: @@ -562,16 +576,20 @@ def deprecated(key, replacement_key=None, invalidation_version=None, warning = ("The '{key}' option (with value '{value}') is deprecated," " please remove it from your configuration.") - def check_for_invalid_version(value): - """Raise error if current version has reach invalidation.""" + def check_for_invalid_version(value: Optional[str]): + """Raise error if current version has reached invalidation.""" if not invalidation_version: return major_version, minor_version, patch_version = \ map(int, invalidation_version.split('.', 2)) + # PATCH_VERSION can be a string when running dev/betas, int otherwise + running_patch_version = (int(PATCH_VERSION.split('.', 1)[0]) + if isinstance(PATCH_VERSION, str) + else PATCH_VERSION) if (MAJOR_VERSION >= major_version and MINOR_VERSION >= minor_version - and PATCH_VERSION >= patch_version): + and running_patch_version >= patch_version): raise vol.Invalid( warning.format( key=key, @@ -581,7 +599,7 @@ def check_for_invalid_version(value): ) ) - def validator(config): + def validator(config: {}): """Check if key is in config and log warning.""" if key in config: value = config[key] @@ -597,7 +615,9 @@ def validator(config): config.pop(key) else: value = default - if replacement_key: + if (replacement_key + and replacement_key not in config + and value is not None): config[replacement_key] = value return has_at_most_one_key(key, replacement_key)(config) return config diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index 8bdb25c913945a..eb410d7e4182a9 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -489,72 +489,304 @@ def test_datetime(): schema('2016-11-23T18:59:08') -def test_deprecated_logging(caplog): - """Test deprecation logs the correct message in each situation.""" - schema = vol.Schema({ +@pytest.fixture +def schema(): + """Create a schema used for testing deprecation.""" + return vol.Schema({ 'venus': cv.boolean, 'mars': cv.boolean, 'jupiter': cv.boolean }) + +def test_deprecated_with_no_optionals(caplog, schema): + """ + Test deprecation behaves correctly when optional params are None. + + Expected behavior: + - Outputs the appropriate deprecation warning if key is detected + - Processes schema without changing any values + - No warning or difference in output if key is not provided + """ deprecated_schema = vol.All( cv.deprecated('mars'), schema ) - deprecated_schema_with_invalidation_version = vol.All( - cv.deprecated('mars', invalidation_version='1.0.0'), + test_data = {'mars': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 1 + assert caplog.records[0].name == __name__ + assert ("The 'mars' option (with value 'True') is deprecated, " + "please remove it from your configuration.") in caplog.text + assert test_data == output + + caplog.clear() + assert len(caplog.records) == 0 + + test_data = {'venus': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 0 + assert test_data == output + + +def test_deprecated_with_replacement_key(caplog, schema): + """ + Test deprecation behaves correctly when only a replacement key is provided. + + Expected behavior: + - Outputs the appropriate deprecation warning if key is detected + - Processes schema moving the value from key to replacement_key + - Processes schema changing nothing if only replacement_key provided + - No warning if only replacement_key provided + - No warning or difference in output if neither key nor + replacement_key are provided + """ + deprecated_schema = vol.All( + cv.deprecated('mars', replacement_key='jupiter'), schema ) - deprecated_schema_with_replacement_key = vol.All( - cv.deprecated('mars', replacement_key='jupiter'), + test_data = {'mars': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 1 + assert ("The 'mars' option (with value 'True') is deprecated, " + "please replace it with 'jupiter'.") in caplog.text + assert {'jupiter': True} == output + + caplog.clear() + assert len(caplog.records) == 0 + + test_data = {'jupiter': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 0 + assert test_data == output + + test_data = {'venus': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 0 + assert test_data == output + + +def test_deprecated_with_invalidation_version(caplog, schema): + """ + Test deprecation behaves correctly with only an invalidation_version. + + Expected behavior: + - Outputs the appropriate deprecation warning if key is detected + - Processes schema without changing any values + - No warning or difference in output if key is not provided + - Once the invalidation_version is crossed, raises vol.Invalid if key + is detected + """ + deprecated_schema = vol.All( + cv.deprecated('mars', invalidation_version='1.0.0'), schema ) - deprecated_schema_with_invalidation_version_and_replacement_key = vol.All( + test_data = {'mars': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 1 + assert ("The 'mars' option (with value 'True') is deprecated, " + "please remove it from your configuration. " + "This option will become invalid in version 1.0.0.") in caplog.text + assert test_data == output + + caplog.clear() + assert len(caplog.records) == 0 + + test_data = {'venus': False} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 0 + assert test_data == output + + invalidated_schema = vol.All( + cv.deprecated('mars', invalidation_version='0.1.0'), + schema + ) + test_data = {'mars': True} + with pytest.raises(vol.MultipleInvalid): + invalidated_schema(test_data) + + +def test_deprecated_with_replacement_key_and_invalidation_version(caplog, + schema): + """ + Test deprecation behaves with a replacement key & invalidation_version. + + Expected behavior: + - Outputs the appropriate deprecation warning if key is detected + - Processes schema moving the value from key to replacement_key + - Processes schema changing nothing if only replacement_key provided + - No warning if only replacement_key provided + - No warning or difference in output if neither key nor + replacement_key are provided + - Once the invalidation_version is crossed, raises vol.Invalid if key + is detected + """ + deprecated_schema = vol.All( cv.deprecated( 'mars', replacement_key='jupiter', invalidation_version='1.0.0' ), schema ) - deprecated_schema({'venus': True}) + test_data = {'mars': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 1 + assert ("The 'mars' option (with value 'True') is deprecated, " + "please replace it with 'jupiter'. This option will become " + "invalid in version 1.0.0.") in caplog.text + assert {'jupiter': True} == output + + caplog.clear() + assert len(caplog.records) == 0 + + test_data = {'jupiter': True} + output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 0 + assert test_data == output + + test_data = {'venus': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 0 + assert test_data == output + + invalidated_schema = vol.All( + cv.deprecated( + 'mars', replacement_key='jupiter', invalidation_version='0.1.0' + ), + schema + ) + test_data = {'mars': True} + with pytest.raises(vol.MultipleInvalid): + invalidated_schema(test_data) + - deprecated_schema({'mars': True}) +def test_deprecated_with_default(caplog, schema): + """ + Test deprecation behaves correctly with a default value. + + This is likely a scenario that would never occur. + + Expected behavior: + - Behaves identically as when the default value was not present + """ + deprecated_schema = vol.All( + cv.deprecated('mars', default=False), + schema + ) + + test_data = {'mars': True} + output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 1 assert caplog.records[0].name == __name__ assert ("The 'mars' option (with value 'True') is deprecated, " "please remove it from your configuration.") in caplog.text + assert test_data == output + caplog.clear() assert len(caplog.records) == 0 - deprecated_schema_with_invalidation_version({'mars': True}) - assert len(caplog.records) == 1 - assert ("The 'mars' option (with value 'True') is deprecated, " - "please remove it from your configuration. " - "This option will become invalid in version 1.0.0.") in caplog.text - caplog.clear() + test_data = {'venus': True} + output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 0 + assert test_data == output + - deprecated_schema_with_replacement_key({'mars': True}) +def test_deprecated_with_replacement_key_and_default(caplog, schema): + """ + Test deprecation behaves correctly when only a replacement key is provided. + + Expected behavior: + - Outputs the appropriate deprecation warning if key is detected + - Processes schema moving the value from key to replacement_key + - Processes schema changing nothing if only replacement_key provided + - No warning if only replacement_key provided + - No warning if neither key nor replacement_key are provided + - Adds replacement_key with default value in this case + """ + deprecated_schema = vol.All( + cv.deprecated('mars', replacement_key='jupiter', default=False), + schema + ) + + test_data = {'mars': True} + output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 1 assert ("The 'mars' option (with value 'True') is deprecated, " "please replace it with 'jupiter'.") in caplog.text + assert {'jupiter': True} == output + caplog.clear() assert len(caplog.records) == 0 - deprecated_schema_with_invalidation_version_and_replacement_key( - {'mars': True} + test_data = {'jupiter': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 0 + assert test_data == output + + test_data = {'venus': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 0 + assert {'venus': True, 'jupiter': False} == output + + +def test_deprecated_with_replacement_key_invalidation_version_default( + caplog, schema +): + """ + Test deprecation with a replacement key, invalidation_version & default. + + Expected behavior: + - Outputs the appropriate deprecation warning if key is detected + - Processes schema moving the value from key to replacement_key + - Processes schema changing nothing if only replacement_key provided + - No warning if only replacement_key provided + - No warning if neither key nor replacement_key are provided + - Adds replacement_key with default value in this case + - Once the invalidation_version is crossed, raises vol.Invalid if key + is detected + """ + deprecated_schema = vol.All( + cv.deprecated( + 'mars', replacement_key='jupiter', invalidation_version='1.0.0', + default=False + ), + schema ) + + test_data = {'mars': True} + output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 1 assert ("The 'mars' option (with value 'True') is deprecated, " "please replace it with 'jupiter'. This option will become " "invalid in version 1.0.0.") in caplog.text + assert {'jupiter': True} == output + caplog.clear() assert len(caplog.records) == 0 + test_data = {'jupiter': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 0 + assert test_data == output + + test_data = {'venus': True} + output = deprecated_schema(test_data.copy()) + assert len(caplog.records) == 0 + assert {'venus': True, 'jupiter': False} == output + + invalidated_schema = vol.All( + cv.deprecated( + 'mars', replacement_key='jupiter', invalidation_version='0.1.0' + ), + schema + ) + test_data = {'mars': True} + with pytest.raises(vol.MultipleInvalid): + invalidated_schema(test_data) + def test_key_dependency(): """Test key_dependency validator.""" From c4d6f3e9b5643e2d7ae2491540a04562f1dd4c8a Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Tue, 29 Jan 2019 22:52:29 -0800 Subject: [PATCH 05/16] Always check has_at_most_one_key --- homeassistant/helpers/config_validation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 294d974396157c..cf214be04a9ce8 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -619,8 +619,8 @@ def validator(config: {}): and replacement_key not in config and value is not None): config[replacement_key] = value - return has_at_most_one_key(key, replacement_key)(config) - return config + + return has_at_most_one_key(key, replacement_key)(config) # Adapted from: https://stackoverflow.com/a/24683360/2267718 class BraceMessage: From 47a53fe418d58e3778f58b3ab2b5449912a69b4d Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Wed, 30 Jan 2019 09:53:12 -0800 Subject: [PATCH 06/16] Fix typing --- homeassistant/helpers/config_validation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index cf214be04a9ce8..8c94daa6087032 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -576,7 +576,7 @@ def deprecated(key: str, warning = ("The '{key}' option (with value '{value}') is deprecated," " please remove it from your configuration.") - def check_for_invalid_version(value: Optional[str]): + def check_for_invalid_version(value: Optional[Any]): """Raise error if current version has reached invalidation.""" if not invalidation_version: return @@ -599,7 +599,7 @@ def check_for_invalid_version(value: Optional[str]): ) ) - def validator(config: {}): + def validator(config: Dict): """Check if key is in config and log warning.""" if key in config: value = config[key] From fc8e2ae94649e67287f48c94adaf4f972b37032c Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Thu, 31 Jan 2019 09:33:15 -0800 Subject: [PATCH 07/16] Move logging helpers to homea new logging helper --- homeassistant/helpers/config_validation.py | 44 +------------------ homeassistant/helpers/logging.py | 49 ++++++++++++++++++++++ 2 files changed, 51 insertions(+), 42 deletions(-) create mode 100644 homeassistant/helpers/logging.py diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 8c94daa6087032..e46361be846cf0 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -21,6 +21,7 @@ from homeassistant.core import valid_entity_id, split_entity_id from homeassistant.exceptions import TemplateError import homeassistant.util.dt as dt_util +from homeassistant.helpers.logging import BraceStyleAdapter from homeassistant.util import slugify as util_slugify from homeassistant.helpers import template as template_helper @@ -604,7 +605,7 @@ def validator(config: Dict): if key in config: value = config[key] check_for_invalid_version(value) - StyleAdapter(logging.getLogger(module_name)).warning( + BraceStyleAdapter(logging.getLogger(module_name)).warning( warning, key=key, value=value, @@ -622,47 +623,6 @@ def validator(config: Dict): return has_at_most_one_key(key, replacement_key)(config) - # Adapted from: https://stackoverflow.com/a/24683360/2267718 - class BraceMessage: - """Represents a logging message with brace style arguments.""" - - def __init__(self, fmt, args, kwargs): - """Initialize a new BraceMessage object.""" - self._fmt = fmt - self._args = args - self._kwargs = kwargs - - def __str__(self): - """Convert the object to a string for logging.""" - return str(self._fmt).format(*self._args, **self._kwargs) - - class StyleAdapter(logging.LoggerAdapter): - """Represents an adapter wrapping the logger allowing BraceMessages.""" - - def __init__(self, logger, extra=None): - """Initialize a new StyleAdapter for the provided logger.""" - super(StyleAdapter, self).__init__(logger, extra or {}) - - def log(self, level, msg, *args, **kwargs): - """Log the message provided at the appropriate level.""" - if self.isEnabledFor(level): - msg, log_kwargs = self.process(msg, kwargs) - self.logger._log( # pylint: disable=protected-access - level, BraceMessage(msg, args, kwargs), (), **log_kwargs - ) - - def process(self, msg, kwargs): - """Process the keyward args in preparation for logging.""" - return ( - msg, - { - key: kwargs[key] - for k in inspect.getfullargspec( - self.logger._log # pylint: disable=protected-access - ).args[1:] if k in kwargs - } - ) - return validator diff --git a/homeassistant/helpers/logging.py b/homeassistant/helpers/logging.py new file mode 100644 index 00000000000000..f05aea25a935ff --- /dev/null +++ b/homeassistant/helpers/logging.py @@ -0,0 +1,49 @@ +"""Helpers for logging allowing more advanced logging styles to be used.""" +import inspect +import logging + + +class BraceMessage: + """ + Represents a logging message with brace style arguments. + + Adapted from: https://stackoverflow.com/a/24683360/2267718 + """ + + def __init__(self, fmt, args, kwargs): + """Initialize a new BraceMessage object.""" + self._fmt = fmt + self._args = args + self._kwargs = kwargs + + def __str__(self): + """Convert the object to a string for logging.""" + return str(self._fmt).format(*self._args, **self._kwargs) + + +class BraceStyleAdapter(logging.LoggerAdapter): + """Represents an adapter wrapping the logger allowing BraceMessages.""" + + def __init__(self, logger, extra=None): + """Initialize a new StyleAdapter for the provided logger.""" + super(BraceStyleAdapter, self).__init__(logger, extra or {}) + + def log(self, level, msg, *args, **kwargs): + """Log the message provided at the appropriate level.""" + if self.isEnabledFor(level): + msg, log_kwargs = self.process(msg, kwargs) + self.logger._log( # pylint: disable=protected-access + level, BraceMessage(msg, args, kwargs), (), **log_kwargs + ) + + def process(self, msg, kwargs): + """Process the keyward args in preparation for logging.""" + return ( + msg, + { + k: kwargs[k] + for k in inspect.getfullargspec( + self.logger._log # pylint: disable=protected-access + ).args[1:] if k in kwargs + } + ) \ No newline at end of file From aa3fe75ebe6deeb38622f7b2789bbd330ebd5d52 Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Thu, 31 Jan 2019 09:34:02 -0800 Subject: [PATCH 08/16] Lint --- homeassistant/helpers/logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/helpers/logging.py b/homeassistant/helpers/logging.py index f05aea25a935ff..7dde0efb84f763 100644 --- a/homeassistant/helpers/logging.py +++ b/homeassistant/helpers/logging.py @@ -46,4 +46,4 @@ def process(self, msg, kwargs): self.logger._log # pylint: disable=protected-access ).args[1:] if k in kwargs } - ) \ No newline at end of file + ) From 34f461dd49936884f0765da16d55e0f04b59b63e Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Sat, 2 Feb 2019 08:58:59 -0800 Subject: [PATCH 09/16] Rename to KeywordMessage instead of BraceMessage --- homeassistant/helpers/config_validation.py | 4 ++-- homeassistant/helpers/logging.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index e46361be846cf0..9e9ac2317bd936 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -21,7 +21,7 @@ from homeassistant.core import valid_entity_id, split_entity_id from homeassistant.exceptions import TemplateError import homeassistant.util.dt as dt_util -from homeassistant.helpers.logging import BraceStyleAdapter +from homeassistant.helpers.logging import KeywordStyleAdapter from homeassistant.util import slugify as util_slugify from homeassistant.helpers import template as template_helper @@ -605,7 +605,7 @@ def validator(config: Dict): if key in config: value = config[key] check_for_invalid_version(value) - BraceStyleAdapter(logging.getLogger(module_name)).warning( + KeywordStyleAdapter(logging.getLogger(module_name)).warning( warning, key=key, value=value, diff --git a/homeassistant/helpers/logging.py b/homeassistant/helpers/logging.py index 7dde0efb84f763..ea596eb3c158ee 100644 --- a/homeassistant/helpers/logging.py +++ b/homeassistant/helpers/logging.py @@ -3,9 +3,9 @@ import logging -class BraceMessage: +class KeywordMessage: """ - Represents a logging message with brace style arguments. + Represents a logging message with keyword arguments. Adapted from: https://stackoverflow.com/a/24683360/2267718 """ @@ -21,19 +21,19 @@ def __str__(self): return str(self._fmt).format(*self._args, **self._kwargs) -class BraceStyleAdapter(logging.LoggerAdapter): - """Represents an adapter wrapping the logger allowing BraceMessages.""" +class KeywordStyleAdapter(logging.LoggerAdapter): + """Represents an adapter wrapping the logger allowing KeywordMessages.""" def __init__(self, logger, extra=None): """Initialize a new StyleAdapter for the provided logger.""" - super(BraceStyleAdapter, self).__init__(logger, extra or {}) + super(KeywordStyleAdapter, self).__init__(logger, extra or {}) def log(self, level, msg, *args, **kwargs): """Log the message provided at the appropriate level.""" if self.isEnabledFor(level): msg, log_kwargs = self.process(msg, kwargs) self.logger._log( # pylint: disable=protected-access - level, BraceMessage(msg, args, kwargs), (), **log_kwargs + level, KeywordMessage(msg, args, kwargs), (), **log_kwargs ) def process(self, msg, kwargs): From af54e3ee9bea0075b14c60781d98e3f9cde15b94 Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Sat, 2 Feb 2019 18:10:27 -0800 Subject: [PATCH 10/16] Remove unneeded KeywordStyleAdapter --- homeassistant/helpers/config_validation.py | 56 +++++++++++----------- homeassistant/helpers/logging.py | 49 ------------------- 2 files changed, 27 insertions(+), 78 deletions(-) delete mode 100644 homeassistant/helpers/logging.py diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 9e9ac2317bd936..f174d8593acf23 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -1,16 +1,17 @@ """Helpers for config validation using voluptuous.""" -from datetime import (timedelta, datetime as datetime_sys, - time as time_sys, date as date_sys) +import inspect +import logging import os import re -from urllib.parse import urlparse +from datetime import (timedelta, datetime as datetime_sys, + time as time_sys, date as date_sys) from socket import _GLOBAL_DEFAULT_TIMEOUT -import logging -import inspect from typing import Any, Union, TypeVar, Callable, Sequence, Dict, Optional +from urllib.parse import urlparse import voluptuous as vol +import homeassistant.util.dt as dt_util from homeassistant.const import ( CONF_PLATFORM, CONF_SCAN_INTERVAL, TEMP_CELSIUS, TEMP_FAHRENHEIT, CONF_ALIAS, CONF_ENTITY_ID, CONF_VALUE_TEMPLATE, WEEKDAYS, @@ -20,10 +21,8 @@ PATCH_VERSION) from homeassistant.core import valid_entity_id, split_entity_id from homeassistant.exceptions import TemplateError -import homeassistant.util.dt as dt_util -from homeassistant.helpers.logging import KeywordStyleAdapter -from homeassistant.util import slugify as util_slugify from homeassistant.helpers import template as template_helper +from homeassistant.util import slugify as util_slugify # pylint: disable=invalid-name @@ -76,12 +75,7 @@ def validate(obj: Dict) -> Dict: if not isinstance(obj, dict): raise vol.Invalid('expected dictionary') - count = 0 - for k in obj.keys(): - if k in keys: - count += 1 - - if count > 1: + if len(set(keys) & set(obj.keys())) > 1: raise vol.Invalid( 'must contain at most one of {}.'.format(', '.join(keys)) ) @@ -563,19 +557,21 @@ def deprecated(key: str, module_name = inspect.getmodule(inspect.stack()[1][0]).__name__ if replacement_key and invalidation_version: - warning = ("The '{key}' option (with value '{value}') is deprecated," - " please replace it with '{replacement_key}'. This option" - " will become invalid in version {invalidation_version}.") + warning = ("The '%(key)s' option (with value '%(value)s') is" + " deprecated, please replace it with '%(replacement_key)s'." + " This option will become invalid in version" + " %(invalidation_version)s.") elif replacement_key: - warning = ("The '{key}' option (with value '{value}') is deprecated," - " please replace it with '{replacement_key}'.") + warning = ("The '%(key)s' option (with value '%(value)s') is" + " deprecated, please replace it with '%(replacement_key)s'.") elif invalidation_version: - warning = ("The '{key}' option (with value '{value}') is deprecated," - " please remove it from your configuration. This option" - " will become invalid in version {invalidation_version}.") + warning = ("The '%(key)s' option (with value '%(value)s') is" + " deprecated, please remove it from your configuration." + " This option will become invalid in version" + " %(invalidation_version)s.") else: - warning = ("The '{key}' option (with value '{value}') is deprecated," - " please remove it from your configuration.") + warning = ("The '%(key)s' option (with value '%(value)s') is" + " deprecated, please remove it from your configuration.") def check_for_invalid_version(value: Optional[Any]): """Raise error if current version has reached invalidation.""" @@ -605,12 +601,14 @@ def validator(config: Dict): if key in config: value = config[key] check_for_invalid_version(value) - KeywordStyleAdapter(logging.getLogger(module_name)).warning( + logging.getLogger(module_name).warning( warning, - key=key, - value=value, - replacement_key=replacement_key, - invalidation_version=invalidation_version + { + 'key': key, + 'value': value, + 'replacement_key': replacement_key, + 'invalidation_version': invalidation_version + } ) if replacement_key: config.pop(key) diff --git a/homeassistant/helpers/logging.py b/homeassistant/helpers/logging.py deleted file mode 100644 index ea596eb3c158ee..00000000000000 --- a/homeassistant/helpers/logging.py +++ /dev/null @@ -1,49 +0,0 @@ -"""Helpers for logging allowing more advanced logging styles to be used.""" -import inspect -import logging - - -class KeywordMessage: - """ - Represents a logging message with keyword arguments. - - Adapted from: https://stackoverflow.com/a/24683360/2267718 - """ - - def __init__(self, fmt, args, kwargs): - """Initialize a new BraceMessage object.""" - self._fmt = fmt - self._args = args - self._kwargs = kwargs - - def __str__(self): - """Convert the object to a string for logging.""" - return str(self._fmt).format(*self._args, **self._kwargs) - - -class KeywordStyleAdapter(logging.LoggerAdapter): - """Represents an adapter wrapping the logger allowing KeywordMessages.""" - - def __init__(self, logger, extra=None): - """Initialize a new StyleAdapter for the provided logger.""" - super(KeywordStyleAdapter, self).__init__(logger, extra or {}) - - def log(self, level, msg, *args, **kwargs): - """Log the message provided at the appropriate level.""" - if self.isEnabledFor(level): - msg, log_kwargs = self.process(msg, kwargs) - self.logger._log( # pylint: disable=protected-access - level, KeywordMessage(msg, args, kwargs), (), **log_kwargs - ) - - def process(self, msg, kwargs): - """Process the keyward args in preparation for logging.""" - return ( - msg, - { - k: kwargs[k] - for k in inspect.getfullargspec( - self.logger._log # pylint: disable=protected-access - ).args[1:] if k in kwargs - } - ) From 4764e769ac98f3693c87cb2cfe7f8fec0cb93d67 Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Sat, 2 Feb 2019 18:11:32 -0800 Subject: [PATCH 11/16] Lint --- homeassistant/helpers/config_validation.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index f174d8593acf23..882e5a491da54f 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -563,7 +563,8 @@ def deprecated(key: str, " %(invalidation_version)s.") elif replacement_key: warning = ("The '%(key)s' option (with value '%(value)s') is" - " deprecated, please replace it with '%(replacement_key)s'.") + " deprecated, please replace it with '%(replacement_key)s'." + ) elif invalidation_version: warning = ("The '%(key)s' option (with value '%(value)s') is" " deprecated, please remove it from your configuration." From e306bdae49d7a9a2b12b7ff81dde233aafa6477f Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Sat, 2 Feb 2019 23:18:27 -0800 Subject: [PATCH 12/16] Use dict directly rather than dict.keys() when creating set --- homeassistant/helpers/config_validation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 882e5a491da54f..3fbb89838c6b8d 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -75,7 +75,7 @@ def validate(obj: Dict) -> Dict: if not isinstance(obj, dict): raise vol.Invalid('expected dictionary') - if len(set(keys) & set(obj.keys())) > 1: + if len(set(keys) & set(obj)) > 1: raise vol.Invalid( 'must contain at most one of {}.'.format(', '.join(keys)) ) From 851ec1a91b92052ed953d97f293c0dcb5ebeb4b2 Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Sun, 3 Feb 2019 10:01:18 -0800 Subject: [PATCH 13/16] Patch the version in unit tests, update logging and use parse_version --- homeassistant/helpers/config_validation.py | 23 +++------ tests/helpers/test_config_validation.py | 60 +++++++++++++--------- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 3fbb89838c6b8d..6691bbdbcab92d 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -10,6 +10,7 @@ from urllib.parse import urlparse import voluptuous as vol +from pkg_resources import parse_version import homeassistant.util.dt as dt_util from homeassistant.const import ( @@ -17,8 +18,7 @@ CONF_ALIAS, CONF_ENTITY_ID, CONF_VALUE_TEMPLATE, WEEKDAYS, CONF_CONDITION, CONF_BELOW, CONF_ABOVE, CONF_TIMEOUT, SUN_EVENT_SUNSET, SUN_EVENT_SUNRISE, CONF_UNIT_SYSTEM_IMPERIAL, CONF_UNIT_SYSTEM_METRIC, - ENTITY_MATCH_ALL, CONF_ENTITY_NAMESPACE, MAJOR_VERSION, MINOR_VERSION, - PATCH_VERSION) + ENTITY_MATCH_ALL, CONF_ENTITY_NAMESPACE, __version__) from homeassistant.core import valid_entity_id, split_entity_id from homeassistant.exceptions import TemplateError from homeassistant.helpers import template as template_helper @@ -560,34 +560,25 @@ def deprecated(key: str, warning = ("The '%(key)s' option (with value '%(value)s') is" " deprecated, please replace it with '%(replacement_key)s'." " This option will become invalid in version" - " %(invalidation_version)s.") + " %(invalidation_version)s") elif replacement_key: warning = ("The '%(key)s' option (with value '%(value)s') is" - " deprecated, please replace it with '%(replacement_key)s'." - ) + " deprecated, please replace it with '%(replacement_key)s'") elif invalidation_version: warning = ("The '%(key)s' option (with value '%(value)s') is" " deprecated, please remove it from your configuration." " This option will become invalid in version" - " %(invalidation_version)s.") + " %(invalidation_version)s") else: warning = ("The '%(key)s' option (with value '%(value)s') is" - " deprecated, please remove it from your configuration.") + " deprecated, please remove it from your configuration") def check_for_invalid_version(value: Optional[Any]): """Raise error if current version has reached invalidation.""" if not invalidation_version: return - major_version, minor_version, patch_version = \ - map(int, invalidation_version.split('.', 2)) - # PATCH_VERSION can be a string when running dev/betas, int otherwise - running_patch_version = (int(PATCH_VERSION.split('.', 1)[0]) - if isinstance(PATCH_VERSION, str) - else PATCH_VERSION) - if (MAJOR_VERSION >= major_version - and MINOR_VERSION >= minor_version - and running_patch_version >= patch_version): + if parse_version(__version__) >= parse_version(invalidation_version): raise vol.Invalid( warning.format( key=key, diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index eb410d7e4182a9..e3319c9287affb 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -5,6 +5,7 @@ from socket import _GLOBAL_DEFAULT_TIMEOUT from unittest.mock import Mock, patch +import homeassistant import pytest import voluptuous as vol @@ -76,7 +77,7 @@ def test_isfile(): # patching methods that allow us to fake a file existing # with write access with patch('os.path.isfile', Mock(return_value=True)), \ - patch('os.access', Mock(return_value=True)): + patch('os.access', Mock(return_value=True)): schema('test.txt') @@ -275,7 +276,6 @@ def test_time_period(): {}, {'wrong_key': -10} ) for value in options: - with pytest.raises(vol.MultipleInvalid): schema(value) @@ -499,6 +499,12 @@ def schema(): }) +@pytest.fixture +def version(monkeypatch): + """Patch the version used for testing to 0.5.0.""" + monkeypatch.setattr(homeassistant.const, '__version__', '0.5.0') + + def test_deprecated_with_no_optionals(caplog, schema): """ Test deprecation behaves correctly when optional params are None. @@ -518,7 +524,7 @@ def test_deprecated_with_no_optionals(caplog, schema): assert len(caplog.records) == 1 assert caplog.records[0].name == __name__ assert ("The 'mars' option (with value 'True') is deprecated, " - "please remove it from your configuration.") in caplog.text + "please remove it from your configuration") in caplog.text assert test_data == output caplog.clear() @@ -551,7 +557,7 @@ def test_deprecated_with_replacement_key(caplog, schema): output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 1 assert ("The 'mars' option (with value 'True') is deprecated, " - "please replace it with 'jupiter'.") in caplog.text + "please replace it with 'jupiter'") in caplog.text assert {'jupiter': True} == output caplog.clear() @@ -568,7 +574,7 @@ def test_deprecated_with_replacement_key(caplog, schema): assert test_data == output -def test_deprecated_with_invalidation_version(caplog, schema): +def test_deprecated_with_invalidation_version(caplog, schema, version): """ Test deprecation behaves correctly with only an invalidation_version. @@ -584,12 +590,14 @@ def test_deprecated_with_invalidation_version(caplog, schema): schema ) + message = ("The 'mars' option (with value 'True') is deprecated, " + "please remove it from your configuration. " + "This option will become invalid in version 1.0.0") + test_data = {'mars': True} output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 1 - assert ("The 'mars' option (with value 'True') is deprecated, " - "please remove it from your configuration. " - "This option will become invalid in version 1.0.0.") in caplog.text + assert message in caplog.text assert test_data == output caplog.clear() @@ -605,12 +613,13 @@ def test_deprecated_with_invalidation_version(caplog, schema): schema ) test_data = {'mars': True} - with pytest.raises(vol.MultipleInvalid): + with pytest.raises(vol.MultipleInvalid, match=message): invalidated_schema(test_data) -def test_deprecated_with_replacement_key_and_invalidation_version(caplog, - schema): +def test_deprecated_with_replacement_key_and_invalidation_version( + caplog, schema, version +): """ Test deprecation behaves with a replacement key & invalidation_version. @@ -631,12 +640,14 @@ def test_deprecated_with_replacement_key_and_invalidation_version(caplog, schema ) + warning = ("The 'mars' option (with value 'True') is deprecated, " + "please replace it with 'jupiter'. This option will become " + "invalid in version 1.0.0") + test_data = {'mars': True} output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 1 - assert ("The 'mars' option (with value 'True') is deprecated, " - "please replace it with 'jupiter'. This option will become " - "invalid in version 1.0.0.") in caplog.text + assert warning in caplog.text assert {'jupiter': True} == output caplog.clear() @@ -659,7 +670,7 @@ def test_deprecated_with_replacement_key_and_invalidation_version(caplog, schema ) test_data = {'mars': True} - with pytest.raises(vol.MultipleInvalid): + with pytest.raises(vol.MultipleInvalid, match=warning): invalidated_schema(test_data) @@ -682,7 +693,7 @@ def test_deprecated_with_default(caplog, schema): assert len(caplog.records) == 1 assert caplog.records[0].name == __name__ assert ("The 'mars' option (with value 'True') is deprecated, " - "please remove it from your configuration.") in caplog.text + "please remove it from your configuration") in caplog.text assert test_data == output caplog.clear() @@ -715,7 +726,7 @@ def test_deprecated_with_replacement_key_and_default(caplog, schema): output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 1 assert ("The 'mars' option (with value 'True') is deprecated, " - "please replace it with 'jupiter'.") in caplog.text + "please replace it with 'jupiter'") in caplog.text assert {'jupiter': True} == output caplog.clear() @@ -733,7 +744,7 @@ def test_deprecated_with_replacement_key_and_default(caplog, schema): def test_deprecated_with_replacement_key_invalidation_version_default( - caplog, schema + caplog, schema, version ): """ Test deprecation with a replacement key, invalidation_version & default. @@ -756,12 +767,14 @@ def test_deprecated_with_replacement_key_invalidation_version_default( schema ) + message = ("The 'mars' option (with value 'True') is deprecated, " + "please replace it with 'jupiter'. This option will become " + "invalid in version 1.0.0") + test_data = {'mars': True} output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 1 - assert ("The 'mars' option (with value 'True') is deprecated, " - "please replace it with 'jupiter'. This option will become " - "invalid in version 1.0.0.") in caplog.text + assert message in caplog.text assert {'jupiter': True} == output caplog.clear() @@ -784,7 +797,7 @@ def test_deprecated_with_replacement_key_invalidation_version_default( schema ) test_data = {'mars': True} - with pytest.raises(vol.MultipleInvalid): + with pytest.raises(vol.MultipleInvalid, match=message): invalidated_schema(test_data) @@ -833,6 +846,7 @@ def test_has_at_least_one_key(): def test_enum(): """Test enum validator.""" + class TestEnum(enum.Enum): """Test enum.""" @@ -871,7 +885,7 @@ def test_matches_regex(): schema(" nrtd ") test_str = "This is a test including uiae." - assert(schema(test_str) == test_str) + assert (schema(test_str) == test_str) def test_is_regex(): From 0d0a48a0de527da00d4ebb91d27b884d5f861f6f Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Sun, 3 Feb 2019 10:14:57 -0800 Subject: [PATCH 14/16] Re-add KeywordStyleAdapter and fix tests --- homeassistant/helpers/config_validation.py | 29 +++++++------ homeassistant/helpers/logging.py | 49 ++++++++++++++++++++++ tests/helpers/test_config_validation.py | 23 ++++++---- 3 files changed, 78 insertions(+), 23 deletions(-) create mode 100644 homeassistant/helpers/logging.py diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 6691bbdbcab92d..3b01a01fc96bd3 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -22,6 +22,7 @@ from homeassistant.core import valid_entity_id, split_entity_id from homeassistant.exceptions import TemplateError from homeassistant.helpers import template as template_helper +from homeassistant.helpers.logging import KeywordStyleAdapter from homeassistant.util import slugify as util_slugify # pylint: disable=invalid-name @@ -557,20 +558,20 @@ def deprecated(key: str, module_name = inspect.getmodule(inspect.stack()[1][0]).__name__ if replacement_key and invalidation_version: - warning = ("The '%(key)s' option (with value '%(value)s') is" - " deprecated, please replace it with '%(replacement_key)s'." + warning = ("The '{key}' option (with value '{value}') is" + " deprecated, please replace it with '{replacement_key}'." " This option will become invalid in version" - " %(invalidation_version)s") + " {invalidation_version}") elif replacement_key: - warning = ("The '%(key)s' option (with value '%(value)s') is" - " deprecated, please replace it with '%(replacement_key)s'") + warning = ("The '{key}' option (with value '{value}') is" + " deprecated, please replace it with '{replacement_key}'") elif invalidation_version: - warning = ("The '%(key)s' option (with value '%(value)s') is" + warning = ("The '{key}' option (with value '{value}') is" " deprecated, please remove it from your configuration." " This option will become invalid in version" - " %(invalidation_version)s") + " {invalidation_version}") else: - warning = ("The '%(key)s' option (with value '%(value)s') is" + warning = ("The '{key}' option (with value '{value}') is" " deprecated, please remove it from your configuration") def check_for_invalid_version(value: Optional[Any]): @@ -593,14 +594,12 @@ def validator(config: Dict): if key in config: value = config[key] check_for_invalid_version(value) - logging.getLogger(module_name).warning( + KeywordStyleAdapter(logging.getLogger(module_name)).warning( warning, - { - 'key': key, - 'value': value, - 'replacement_key': replacement_key, - 'invalidation_version': invalidation_version - } + key=key, + value=value, + replacement_key=replacement_key, + invalidation_version=invalidation_version ) if replacement_key: config.pop(key) diff --git a/homeassistant/helpers/logging.py b/homeassistant/helpers/logging.py new file mode 100644 index 00000000000000..ea596eb3c158ee --- /dev/null +++ b/homeassistant/helpers/logging.py @@ -0,0 +1,49 @@ +"""Helpers for logging allowing more advanced logging styles to be used.""" +import inspect +import logging + + +class KeywordMessage: + """ + Represents a logging message with keyword arguments. + + Adapted from: https://stackoverflow.com/a/24683360/2267718 + """ + + def __init__(self, fmt, args, kwargs): + """Initialize a new BraceMessage object.""" + self._fmt = fmt + self._args = args + self._kwargs = kwargs + + def __str__(self): + """Convert the object to a string for logging.""" + return str(self._fmt).format(*self._args, **self._kwargs) + + +class KeywordStyleAdapter(logging.LoggerAdapter): + """Represents an adapter wrapping the logger allowing KeywordMessages.""" + + def __init__(self, logger, extra=None): + """Initialize a new StyleAdapter for the provided logger.""" + super(KeywordStyleAdapter, self).__init__(logger, extra or {}) + + def log(self, level, msg, *args, **kwargs): + """Log the message provided at the appropriate level.""" + if self.isEnabledFor(level): + msg, log_kwargs = self.process(msg, kwargs) + self.logger._log( # pylint: disable=protected-access + level, KeywordMessage(msg, args, kwargs), (), **log_kwargs + ) + + def process(self, msg, kwargs): + """Process the keyward args in preparation for logging.""" + return ( + msg, + { + k: kwargs[k] + for k in inspect.getfullargspec( + self.logger._log # pylint: disable=protected-access + ).args[1:] if k in kwargs + } + ) diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index e3319c9287affb..d644ade4e7c730 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -613,8 +613,11 @@ def test_deprecated_with_invalidation_version(caplog, schema, version): schema ) test_data = {'mars': True} - with pytest.raises(vol.MultipleInvalid, match=message): + with pytest.raises(vol.MultipleInvalid) as exc_info: invalidated_schema(test_data) + assert ("The 'mars' option (with value 'True') is deprecated, " + "please remove it from your configuration. This option will " + "become invalid in version 0.1.0") == str(exc_info.value) def test_deprecated_with_replacement_key_and_invalidation_version( @@ -670,8 +673,11 @@ def test_deprecated_with_replacement_key_and_invalidation_version( schema ) test_data = {'mars': True} - with pytest.raises(vol.MultipleInvalid, match=warning): + with pytest.raises(vol.MultipleInvalid) as exc_info: invalidated_schema(test_data) + assert ("The 'mars' option (with value 'True') is deprecated, " + "please replace it with 'jupiter'. This option will become " + "invalid in version 0.1.0") == str(exc_info.value) def test_deprecated_with_default(caplog, schema): @@ -767,14 +773,12 @@ def test_deprecated_with_replacement_key_invalidation_version_default( schema ) - message = ("The 'mars' option (with value 'True') is deprecated, " - "please replace it with 'jupiter'. This option will become " - "invalid in version 1.0.0") - test_data = {'mars': True} output = deprecated_schema(test_data.copy()) assert len(caplog.records) == 1 - assert message in caplog.text + assert ("The 'mars' option (with value 'True') is deprecated, " + "please replace it with 'jupiter'. This option will become " + "invalid in version 1.0.0") in caplog.text assert {'jupiter': True} == output caplog.clear() @@ -797,8 +801,11 @@ def test_deprecated_with_replacement_key_invalidation_version_default( schema ) test_data = {'mars': True} - with pytest.raises(vol.MultipleInvalid, match=message): + with pytest.raises(vol.MultipleInvalid) as exc_info: invalidated_schema(test_data) + assert ("The 'mars' option (with value 'True') is deprecated, " + "please replace it with 'jupiter'. This option will become " + "invalid in version 0.1.0") == str(exc_info.value) def test_key_dependency(): From cab3d20ff3ef23c692917aa405f7475a0310943b Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Sun, 3 Feb 2019 10:18:59 -0800 Subject: [PATCH 15/16] Lint --- tests/helpers/test_config_validation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index d644ade4e7c730..f36db0fefd0456 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -77,7 +77,7 @@ def test_isfile(): # patching methods that allow us to fake a file existing # with write access with patch('os.path.isfile', Mock(return_value=True)), \ - patch('os.access', Mock(return_value=True)): + patch('os.access', Mock(return_value=True)): schema('test.txt') From 5de4766b271a2277b06a8c9e22a2e561264fc558 Mon Sep 17 00:00:00 2001 From: Rohan Kapoor Date: Sun, 3 Feb 2019 13:13:14 -0800 Subject: [PATCH 16/16] Lint --- tests/helpers/test_config_validation.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index f36db0fefd0456..cefde564035a3a 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -853,7 +853,6 @@ def test_has_at_least_one_key(): def test_enum(): """Test enum validator.""" - class TestEnum(enum.Enum): """Test enum."""