Add better handling of deprecated configs (#20565)

* Add better handling of deprecated configs

* Embed the call to has_at_most_one_key in deprecated

* Add tests for checking the deprecated logs

* Add thoroughly documented tests

* Always check has_at_most_one_key

* Fix typing

* Move logging helpers to homea new logging helper

* Lint

* Rename to KeywordMessage instead of BraceMessage

* Remove unneeded KeywordStyleAdapter

* Lint

* Use dict directly rather than dict.keys() when creating set

* Patch the version in unit tests, update logging and use parse_version

* Re-add KeywordStyleAdapter and fix tests

* Lint

* Lint

homeassistant/components/freedns/__init__.py

@@ -13,7 +13,7 @@ import async_timeout
 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,31 @@ TIMEOUT = 10
 UPDATE_URL = 'https://freedns.afraid.org/dynamic/update.php'
 
 CONFIG_SCHEMA = vol.Schema({
-    DOMAIN: vol.Schema({
+    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, default=DEFAULT_INTERVAL): vol.All(
-            cv.time_period, cv.positive_timedelta),
-
-    })
+            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
+        )
+    )
 }, 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()
 

homeassistant/helpers/config_validation.py

@@ -1,27 +1,29 @@
 """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
+from typing import Any, Union, TypeVar, Callable, Sequence, Dict, Optional
+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 (
     CONF_PLATFORM, CONF_SCAN_INTERVAL, TEMP_CELSIUS, TEMP_FAHRENHEIT,
     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, __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.util import slugify as util_slugify
 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
 
@@ -67,6 +69,22 @@ def has_at_least_one_key(*keys: str) -> Callable:
     return validate
 
 
+def has_at_most_one_key(*keys: str) -> Callable:
+    """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')
+
+        if len(set(keys) & set(obj)) > 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,18 +538,79 @@ def ensure_list_csv(value: Any) -> Sequence:
     return ensure_list(value)
 
 
-def deprecated(key):
-    """Log key as deprecated."""
+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__
 
-    def validator(config):
+    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: Optional[Any]):
+        """Raise error if current version has reached invalidation."""
+        if not invalidation_version:
+            return
+
+        if parse_version(__version__) >= parse_version(invalidation_version):
+            raise vol.Invalid(
+                warning.format(
+                    key=key,
+                    value=value,
+                    replacement_key=replacement_key,
+                    invalidation_version=invalidation_version
+                )
+            )
+
+    def validator(config: Dict):
         """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)
+            KeywordStyleAdapter(logging.getLogger(module_name)).warning(
+                warning,
+                key=key,
+                value=value,
+                replacement_key=replacement_key,
+                invalidation_version=invalidation_version
+            )
+            if replacement_key:
+                config.pop(key)
+        else:
+            value = default
+        if (replacement_key
+                and replacement_key not in config
+                and value is not None):
+            config[replacement_key] = value
 
-        return config
+        return has_at_most_one_key(key, replacement_key)(config)
 
     return validator
 

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
+            }
+        )

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

tests/helpers/test_config_validation.py

@@ -5,6 +5,7 @@ import os
 from socket import _GLOBAL_DEFAULT_TIMEOUT
 from unittest.mock import Mock, patch
 
+import homeassistant
 import pytest
 import voluptuous as vol
 
@@ -275,7 +276,6 @@ def test_time_period():
         {}, {'wrong_key': -10}
     )
     for value in options:
-
         with pytest.raises(vol.MultipleInvalid):
             schema(value)
 
@@ -489,26 +489,323 @@ def test_datetime():
     schema('2016-11-23T18:59:08')
 
 
-def test_deprecated(caplog):
-    """Test deprecation log."""
-    schema = vol.Schema({
+@pytest.fixture
+def schema():
+    """Create a schema used for testing deprecation."""
+    return vol.Schema({
         'venus': cv.boolean,
-        'mars': cv.boolean
+        'mars': cv.boolean,
+        'jupiter': cv.boolean
     })
+
+
+@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.
+
+    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({'venus': True})
-    # pylint: disable=len-as-condition
-    assert len(caplog.records) == 0
-
-    deprecated_schema({'mars': True})
+    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
+            "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
+    )
+
+    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, version):
+    """
+    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
+    )
+
+    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 message 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) 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(
+        caplog, schema, version
+):
+    """
+    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
+    )
+
+    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 warning 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) 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):
+    """
+    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
+
+    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_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
+
+    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, version
+):
+    """
+    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) 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():
@@ -530,6 +827,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'))