From 9c09a98c9e636393608e20c2599ada68da80a3c2 Mon Sep 17 00:00:00 2001 From: Otto Winter Date: Thu, 14 Feb 2019 20:55:51 +0100 Subject: [PATCH 1/5] Add legacy PLATFORM_SCHEMA config validation --- homeassistant/helpers/config_validation.py | 51 +++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 3b01a01fc96..473fcc88717 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -25,6 +25,8 @@ from homeassistant.helpers import template as template_helper from homeassistant.helpers.logging import KeywordStyleAdapter from homeassistant.util import slugify as util_slugify +_LOGGER = logging.getLogger(__name__) + # pylint: disable=invalid-name TIME_PERIOD_ERROR = "offset {} should be format 'HH:MM' or 'HH:MM:SS'" @@ -632,8 +634,55 @@ def key_dependency(key, dependency): # Schemas +class HASchema(vol.Schema): + """Schema class that allows us to mark PREVENT_EXTRA errors as warnings.""" + def __call__(self, data): + try: + return super().__call__(data) + except vol.Invalid as orig_err: + if self.extra != vol.PREVENT_EXTRA: + raise -PLATFORM_SCHEMA = vol.Schema({ + # orig_error is of type vol.MultipleInvalid (see super __call__) + assert isinstance(orig_err, vol.MultipleInvalid) + # If it fails with PREVENT_EXTRA, try with ALLOW_EXTRA + self.extra = vol.ALLOW_EXTRA + # In case it still fails the following will raise + try: + validated = super().__call__(data) + finally: + self.extra = vol.PREVENT_EXTRA + + # This is a legacy config, print warning + extra_key_errs = [err for err in orig_err.errors + if err.error_message == 'extra keys not allowed'] + if extra_key_errs: + msg = "Your configuration contains extra keys " \ + "that the platform does not support. The keys " + msg += ', '.join('[{}]'.format(err.path[-1]) for err in + extra_key_errs) + msg += ' are 42.' + if hasattr(data, '__config_file__'): + msg += " (See {}, line {}). ".format(data.__config_file__, + data.__line__) + _LOGGER.warning(msg) + else: + # This should not happen (all errors should be extra key + # errors). Let's raise the original error anyway. + raise orig_err + + # Return legacy validated config + return validated + + def extend(self, schema, required=None, extra=None): + """Extend this schema and convert it to HASchema if necessary""" + ret = super().extend(schema, required=required, extra=extra) + if extra is not None: + return ret + return HASchema(ret.schema, required=required, extra=self.extra) + + +PLATFORM_SCHEMA = HASchema({ vol.Required(CONF_PLATFORM): string, vol.Optional(CONF_ENTITY_NAMESPACE): string, vol.Optional(CONF_SCAN_INTERVAL): time_period From b7607ff472f5cd70cd9b2e6276ded7f1f5cad975 Mon Sep 17 00:00:00 2001 From: Otto Winter Date: Fri, 15 Feb 2019 10:51:52 +0100 Subject: [PATCH 2/5] Fix tests --- tests/test_setup.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/test_setup.py b/tests/test_setup.py index 8575b023d37..1a60943a72d 100644 --- a/tests/test_setup.py +++ b/tests/test_setup.py @@ -90,7 +90,7 @@ class TestSetup: } }) - def test_validate_platform_config(self): + def test_validate_platform_config(self, caplog): """Test validating platform configuration.""" platform_schema = PLATFORM_SCHEMA.extend({ 'hello': str, @@ -109,7 +109,7 @@ class TestSetup: MockPlatform('whatever', platform_schema=platform_schema)) - with assert_setup_component(0): + with assert_setup_component(1): assert setup.setup_component(self.hass, 'platform_conf', { 'platform_conf': { 'platform': 'whatever', @@ -117,11 +117,12 @@ class TestSetup: 'invalid': 'extra', } }) + assert caplog.text.count('Your configuration contains extra keys') == 1 self.hass.data.pop(setup.DATA_SETUP) self.hass.config.components.remove('platform_conf') - with assert_setup_component(1): + with assert_setup_component(2): assert setup.setup_component(self.hass, 'platform_conf', { 'platform_conf': { 'platform': 'whatever', @@ -132,6 +133,7 @@ class TestSetup: 'invalid': True } }) + assert caplog.text.count('Your configuration contains extra keys') == 2 self.hass.data.pop(setup.DATA_SETUP) self.hass.config.components.remove('platform_conf') @@ -183,7 +185,7 @@ class TestSetup: assert 'platform_conf' in self.hass.config.components assert not config['platform_conf'] # empty - def test_validate_platform_config_2(self): + def test_validate_platform_config_2(self, caplog): """Test component PLATFORM_SCHEMA_BASE prio over PLATFORM_SCHEMA.""" platform_schema = PLATFORM_SCHEMA.extend({ 'hello': str, @@ -204,7 +206,7 @@ class TestSetup: MockPlatform('whatever', platform_schema=platform_schema)) - with assert_setup_component(0): + with assert_setup_component(1): assert setup.setup_component(self.hass, 'platform_conf', { # fail: no extra keys allowed in platform schema 'platform_conf': { @@ -213,6 +215,7 @@ class TestSetup: 'invalid': 'extra', } }) + assert caplog.text.count('Your configuration contains extra keys') == 1 self.hass.data.pop(setup.DATA_SETUP) self.hass.config.components.remove('platform_conf') @@ -234,7 +237,7 @@ class TestSetup: self.hass.data.pop(setup.DATA_SETUP) self.hass.config.components.remove('platform_conf') - def test_validate_platform_config_3(self): + def test_validate_platform_config_3(self, caplog): """Test fallback to component PLATFORM_SCHEMA.""" component_schema = PLATFORM_SCHEMA_BASE.extend({ 'hello': str, @@ -255,15 +258,15 @@ class TestSetup: MockPlatform('whatever', platform_schema=platform_schema)) - with assert_setup_component(0): + with assert_setup_component(1): assert setup.setup_component(self.hass, 'platform_conf', { 'platform_conf': { - # fail: no extra keys allowed 'platform': 'whatever', 'hello': 'world', 'invalid': 'extra', } }) + assert caplog.text.count('Your configuration contains extra keys') == 1 self.hass.data.pop(setup.DATA_SETUP) self.hass.config.components.remove('platform_conf') From f6ae054e9fc884b6a3a6c5dab1e9764a904cbc19 Mon Sep 17 00:00:00 2001 From: Otto Winter Date: Fri, 15 Feb 2019 10:57:02 +0100 Subject: [PATCH 3/5] Lint --- homeassistant/helpers/config_validation.py | 5 ++++- tests/test_setup.py | 12 ++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 473fcc88717..f10cf9e057e 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -636,7 +636,9 @@ def key_dependency(key, dependency): # Schemas class HASchema(vol.Schema): """Schema class that allows us to mark PREVENT_EXTRA errors as warnings.""" + def __call__(self, data): + """Override __call__ to mark PREVENT_EXTRA as warning.""" try: return super().__call__(data) except vol.Invalid as orig_err: @@ -645,6 +647,7 @@ class HASchema(vol.Schema): # orig_error is of type vol.MultipleInvalid (see super __call__) assert isinstance(orig_err, vol.MultipleInvalid) + # pylint: disable=no-member # If it fails with PREVENT_EXTRA, try with ALLOW_EXTRA self.extra = vol.ALLOW_EXTRA # In case it still fails the following will raise @@ -675,7 +678,7 @@ class HASchema(vol.Schema): return validated def extend(self, schema, required=None, extra=None): - """Extend this schema and convert it to HASchema if necessary""" + """Extend this schema and convert it to HASchema if necessary.""" ret = super().extend(schema, required=required, extra=extra) if extra is not None: return ret diff --git a/tests/test_setup.py b/tests/test_setup.py index 1a60943a72d..c6126bc4a3b 100644 --- a/tests/test_setup.py +++ b/tests/test_setup.py @@ -117,7 +117,8 @@ class TestSetup: 'invalid': 'extra', } }) - assert caplog.text.count('Your configuration contains extra keys') == 1 + assert caplog.text.count('Your configuration contains ' + 'extra keys') == 1 self.hass.data.pop(setup.DATA_SETUP) self.hass.config.components.remove('platform_conf') @@ -133,7 +134,8 @@ class TestSetup: 'invalid': True } }) - assert caplog.text.count('Your configuration contains extra keys') == 2 + assert caplog.text.count('Your configuration contains ' + 'extra keys') == 2 self.hass.data.pop(setup.DATA_SETUP) self.hass.config.components.remove('platform_conf') @@ -215,7 +217,8 @@ class TestSetup: 'invalid': 'extra', } }) - assert caplog.text.count('Your configuration contains extra keys') == 1 + assert caplog.text.count('Your configuration contains ' + 'extra keys') == 1 self.hass.data.pop(setup.DATA_SETUP) self.hass.config.components.remove('platform_conf') @@ -266,7 +269,8 @@ class TestSetup: 'invalid': 'extra', } }) - assert caplog.text.count('Your configuration contains extra keys') == 1 + assert caplog.text.count('Your configuration contains ' + 'extra keys') == 1 self.hass.data.pop(setup.DATA_SETUP) self.hass.config.components.remove('platform_conf') From 06f2aa93a4c21677209f7c352d9436dcf6fc2f25 Mon Sep 17 00:00:00 2001 From: Otto Winter Date: Fri, 15 Feb 2019 18:40:46 +0100 Subject: [PATCH 4/5] Add persistent notification --- homeassistant/bootstrap.py | 17 +++++++++++++++++ homeassistant/helpers/config_validation.py | 20 +++++++++++--------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/homeassistant/bootstrap.py b/homeassistant/bootstrap.py index 90a74f23598..7e12a516478 100644 --- a/homeassistant/bootstrap.py +++ b/homeassistant/bootstrap.py @@ -192,6 +192,23 @@ async def async_from_config_dict(config: Dict[str, Any], '\n\n'.join(msg), "Config Warning", "config_warning" ) + # TEMP: warn users for invalid slugs + # Remove after 0.92 + if cv.INVALID_EXTRA_KEYS_FOUND: + msg = [] + msg.append( + "Your configuration contains extra keys " + "that the platform does not support (but were silently " + "accepted before 0.88). Please find and remove the following." + "This will become a breaking change." + ) + msg.append('\n'.join('- {}'.format(it) + for it in cv.INVALID_EXTRA_KEYS_FOUND)) + + hass.components.persistent_notification.async_create( + '\n\n'.join(msg), "Config Warning", "config_warning" + ) + return hass diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index f10cf9e057e..41a55dc38dc 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -25,8 +25,6 @@ from homeassistant.helpers import template as template_helper from homeassistant.helpers.logging import KeywordStyleAdapter from homeassistant.util import slugify as util_slugify -_LOGGER = logging.getLogger(__name__) - # pylint: disable=invalid-name TIME_PERIOD_ERROR = "offset {} should be format 'HH:MM' or 'HH:MM:SS'" @@ -36,6 +34,7 @@ OLD_ENTITY_ID_VALIDATION = r"^(\w+)\.(\w+)$" # persistent notification. Rare temporary exception to use a global. INVALID_SLUGS_FOUND = {} INVALID_ENTITY_IDS_FOUND = {} +INVALID_EXTRA_KEYS_FOUND = [] # Home Assistant types @@ -661,14 +660,17 @@ class HASchema(vol.Schema): if err.error_message == 'extra keys not allowed'] if extra_key_errs: msg = "Your configuration contains extra keys " \ - "that the platform does not support. The keys " - msg += ', '.join('[{}]'.format(err.path[-1]) for err in - extra_key_errs) - msg += ' are 42.' + "that the platform does not support.\n" \ + "Please remove " + submsg = ', '.join('[{}]'.format(err.path[-1]) for err in + extra_key_errs) + submsg += '. ' if hasattr(data, '__config_file__'): - msg += " (See {}, line {}). ".format(data.__config_file__, - data.__line__) - _LOGGER.warning(msg) + submsg += " (See {}, line {}). ".format( + data.__config_file__, data.__line__) + msg += submsg + logging.getLogger(__name__).warning(msg) + INVALID_EXTRA_KEYS_FOUND.append(submsg) else: # This should not happen (all errors should be extra key # errors). Let's raise the original error anyway. From a49686879da6a0286a30ab9fc5870461cebaf447 Mon Sep 17 00:00:00 2001 From: Otto Winter Date: Sat, 16 Feb 2019 14:51:30 +0100 Subject: [PATCH 5/5] Update bootstrap.py --- homeassistant/bootstrap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/bootstrap.py b/homeassistant/bootstrap.py index 7e12a516478..a018d540033 100644 --- a/homeassistant/bootstrap.py +++ b/homeassistant/bootstrap.py @@ -192,7 +192,7 @@ async def async_from_config_dict(config: Dict[str, Any], '\n\n'.join(msg), "Config Warning", "config_warning" ) - # TEMP: warn users for invalid slugs + # TEMP: warn users of invalid extra keys # Remove after 0.92 if cv.INVALID_EXTRA_KEYS_FOUND: msg = []