Remove logic converting empty or falsy YAML to empty dict (#103912)

* Correct logic converting empty YAML to empty dict

* Modify according to github comments

* Add load_yaml_dict helper

* Update check_config script

* Update tests
This commit is contained in:
Erik Montnemery 2023-12-05 18:08:11 +01:00 committed by GitHub
parent a8ca73a7dd
commit 5b55c7da5f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 112 additions and 47 deletions

View File

@ -215,7 +215,7 @@ class DomainBlueprints:
def _load_blueprint(self, blueprint_path) -> Blueprint:
"""Load a blueprint."""
try:
blueprint_data = yaml.load_yaml(self.blueprint_folder / blueprint_path)
blueprint_data = yaml.load_yaml_dict(self.blueprint_folder / blueprint_path)
except FileNotFoundError as err:
raise FailedToLoad(
self.domain,
@ -225,7 +225,6 @@ class DomainBlueprints:
except HomeAssistantError as err:
raise FailedToLoad(self.domain, blueprint_path, err) from err
assert isinstance(blueprint_data, dict)
return Blueprint(
blueprint_data, expected_domain=self.domain, path=blueprint_path
)

View File

@ -14,7 +14,7 @@ from homeassistant.const import CONF_FILENAME
from homeassistant.core import callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import collection, storage
from homeassistant.util.yaml import Secrets, load_yaml
from homeassistant.util.yaml import Secrets, load_yaml_dict
from .const import (
CONF_ICON,
@ -201,7 +201,9 @@ class LovelaceYAML(LovelaceConfig):
is_updated = self._cache is not None
try:
config = load_yaml(self.path, Secrets(Path(self.hass.config.config_dir)))
config = load_yaml_dict(
self.path, Secrets(Path(self.hass.config.config_dir))
)
except FileNotFoundError:
raise ConfigNotFound from None

View File

@ -17,7 +17,7 @@ from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType
from homeassistant.loader import async_get_integration, bind_hass
from homeassistant.setup import async_prepare_setup_platform, async_start_setup
from homeassistant.util import slugify
from homeassistant.util.yaml import load_yaml
from homeassistant.util.yaml import load_yaml_dict
from .const import (
ATTR_DATA,
@ -280,8 +280,8 @@ class BaseNotificationService:
# Load service descriptions from notify/services.yaml
integration = await async_get_integration(hass, DOMAIN)
services_yaml = integration.file_path / "services.yaml"
self.services_dict = cast(
dict, await hass.async_add_executor_job(load_yaml, str(services_yaml))
self.services_dict = await hass.async_add_executor_job(
load_yaml_dict, str(services_yaml)
)
async def async_register_services(self) -> None:

View File

@ -27,7 +27,7 @@ from homeassistant.helpers.typing import ConfigType
from homeassistant.loader import bind_hass
from homeassistant.util import raise_if_invalid_filename
import homeassistant.util.dt as dt_util
from homeassistant.util.yaml.loader import load_yaml
from homeassistant.util.yaml.loader import load_yaml_dict
_LOGGER = logging.getLogger(__name__)
@ -120,7 +120,7 @@ def discover_scripts(hass):
# Load user-provided service descriptions from python_scripts/services.yaml
services_yaml = os.path.join(path, "services.yaml")
if os.path.exists(services_yaml):
services_dict = load_yaml(services_yaml)
services_dict = load_yaml_dict(services_yaml)
else:
services_dict = {}

View File

@ -6,7 +6,7 @@ from collections.abc import Coroutine, Mapping
from functools import partial
import logging
from pathlib import Path
from typing import TYPE_CHECKING, Any, cast
from typing import TYPE_CHECKING, Any
import voluptuous as vol
@ -31,7 +31,7 @@ import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.service import async_set_service_schema
from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType
from homeassistant.setup import async_prepare_setup_platform
from homeassistant.util.yaml import load_yaml
from homeassistant.util.yaml import load_yaml_dict
from .const import (
ATTR_CACHE,
@ -104,8 +104,8 @@ async def async_setup_legacy(
# Load service descriptions from tts/services.yaml
services_yaml = Path(__file__).parent / "services.yaml"
services_dict = cast(
dict, await hass.async_add_executor_job(load_yaml, str(services_yaml))
services_dict = await hass.async_add_executor_job(
load_yaml_dict, str(services_yaml)
)
async def async_setup_platform(

View File

@ -66,7 +66,7 @@ from .loader import ComponentProtocol, Integration, IntegrationNotFound
from .requirements import RequirementsNotFound, async_get_integration_with_requirements
from .util.package import is_docker_env
from .util.unit_system import get_unit_system, validate_unit_system
from .util.yaml import SECRET_YAML, Secrets, load_yaml
from .util.yaml import SECRET_YAML, Secrets, YamlTypeError, load_yaml_dict
_LOGGER = logging.getLogger(__name__)
@ -476,15 +476,15 @@ def load_yaml_config_file(
This method needs to run in an executor.
"""
conf_dict = load_yaml(config_path, secrets)
if not isinstance(conf_dict, dict):
try:
conf_dict = load_yaml_dict(config_path, secrets)
except YamlTypeError as exc:
msg = (
f"The configuration file {os.path.basename(config_path)} "
"does not contain a dictionary"
)
_LOGGER.error(msg)
raise HomeAssistantError(msg)
raise HomeAssistantError(msg) from exc
# Convert values to dictionaries if they are None
for key, value in conf_dict.items():

View File

@ -42,7 +42,7 @@ from homeassistant.exceptions import (
UnknownUser,
)
from homeassistant.loader import Integration, async_get_integrations, bind_hass
from homeassistant.util.yaml import load_yaml
from homeassistant.util.yaml import load_yaml_dict
from homeassistant.util.yaml.loader import JSON_TYPE
from . import (
@ -542,7 +542,9 @@ def _load_services_file(hass: HomeAssistant, integration: Integration) -> JSON_T
try:
return cast(
JSON_TYPE,
_SERVICES_SCHEMA(load_yaml(str(integration.file_path / "services.yaml"))),
_SERVICES_SCHEMA(
load_yaml_dict(str(integration.file_path / "services.yaml"))
),
)
except FileNotFoundError:
_LOGGER.warning(

View File

@ -32,7 +32,7 @@ REQUIREMENTS = ("colorlog==6.7.0",)
_LOGGER = logging.getLogger(__name__)
MOCKS: dict[str, tuple[str, Callable]] = {
"load": ("homeassistant.util.yaml.loader.load_yaml", yaml_loader.load_yaml),
"load*": ("homeassistant.config.load_yaml", yaml_loader.load_yaml),
"load*": ("homeassistant.config.load_yaml_dict", yaml_loader.load_yaml_dict),
"secrets": ("homeassistant.util.yaml.loader.secret_yaml", yaml_loader.secret_yaml),
}

View File

@ -2,7 +2,14 @@
from .const import SECRET_YAML
from .dumper import dump, save_yaml
from .input import UndefinedSubstitution, extract_inputs, substitute
from .loader import Secrets, load_yaml, parse_yaml, secret_yaml
from .loader import (
Secrets,
YamlTypeError,
load_yaml,
load_yaml_dict,
parse_yaml,
secret_yaml,
)
from .objects import Input
__all__ = [
@ -11,7 +18,9 @@ __all__ = [
"dump",
"save_yaml",
"Secrets",
"YamlTypeError",
"load_yaml",
"load_yaml_dict",
"secret_yaml",
"parse_yaml",
"UndefinedSubstitution",

View File

@ -36,6 +36,10 @@ _DictT = TypeVar("_DictT", bound=dict)
_LOGGER = logging.getLogger(__name__)
class YamlTypeError(HomeAssistantError):
"""Raised by load_yaml_dict if top level data is not a dict."""
class Secrets:
"""Store secrets while loading YAML."""
@ -211,7 +215,7 @@ class SafeLineLoader(PythonSafeLoader):
LoaderType = FastSafeLoader | PythonSafeLoader
def load_yaml(fname: str, secrets: Secrets | None = None) -> JSON_TYPE:
def load_yaml(fname: str, secrets: Secrets | None = None) -> JSON_TYPE | None:
"""Load a YAML file."""
try:
with open(fname, encoding="utf-8") as conf_file:
@ -221,6 +225,20 @@ def load_yaml(fname: str, secrets: Secrets | None = None) -> JSON_TYPE:
raise HomeAssistantError(exc) from exc
def load_yaml_dict(fname: str, secrets: Secrets | None = None) -> dict:
"""Load a YAML file and ensure the top level is a dict.
Raise if the top level is not a dict.
Return an empty dict if the file is empty.
"""
loaded_yaml = load_yaml(fname, secrets)
if loaded_yaml is None:
loaded_yaml = {}
if not isinstance(loaded_yaml, dict):
raise YamlTypeError(f"YAML file {fname} does not contain a dict")
return loaded_yaml
def parse_yaml(
content: str | TextIO | StringIO, secrets: Secrets | None = None
) -> JSON_TYPE:
@ -255,12 +273,7 @@ def _parse_yaml(
secrets: Secrets | None = None,
) -> JSON_TYPE:
"""Load a YAML file."""
# If configuration file is empty YAML returns None
# We convert that to an empty dict
return (
yaml.load(content, Loader=lambda stream: loader(stream, secrets)) # type: ignore[arg-type]
or NodeDictClass()
)
return yaml.load(content, Loader=lambda stream: loader(stream, secrets)) # type: ignore[arg-type]
@overload
@ -309,7 +322,10 @@ def _include_yaml(loader: LoaderType, node: yaml.nodes.Node) -> JSON_TYPE:
"""
fname = os.path.join(os.path.dirname(loader.get_name()), node.value)
try:
return _add_reference(load_yaml(fname, loader.secrets), loader, node)
loaded_yaml = load_yaml(fname, loader.secrets)
if loaded_yaml is None:
loaded_yaml = NodeDictClass()
return _add_reference(loaded_yaml, loader, node)
except FileNotFoundError as exc:
raise HomeAssistantError(
f"{node.start_mark}: Unable to read file {fname}."
@ -339,7 +355,10 @@ def _include_dir_named_yaml(loader: LoaderType, node: yaml.nodes.Node) -> NodeDi
filename = os.path.splitext(os.path.basename(fname))[0]
if os.path.basename(fname) == SECRET_YAML:
continue
mapping[filename] = load_yaml(fname, loader.secrets)
loaded_yaml = load_yaml(fname, loader.secrets)
if loaded_yaml is None:
continue
mapping[filename] = loaded_yaml
return _add_reference(mapping, loader, node)
@ -364,9 +383,10 @@ def _include_dir_list_yaml(
"""Load multiple files from directory as a list."""
loc = os.path.join(os.path.dirname(loader.get_name()), node.value)
return [
load_yaml(f, loader.secrets)
loaded_yaml
for f in _find_files(loc, "*.yaml")
if os.path.basename(f) != SECRET_YAML
and (loaded_yaml := load_yaml(f, loader.secrets)) is not None
]

View File

@ -13,7 +13,7 @@ from voluptuous.humanize import humanize_error
from homeassistant.const import CONF_SELECTOR
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import config_validation as cv, selector, service
from homeassistant.util.yaml import load_yaml
from homeassistant.util.yaml import load_yaml_dict
from .model import Config, Integration
@ -107,7 +107,7 @@ def grep_dir(path: pathlib.Path, glob_pattern: str, search_pattern: str) -> bool
def validate_services(config: Config, integration: Integration) -> None:
"""Validate services."""
try:
data = load_yaml(str(integration.path / "services.yaml"))
data = load_yaml_dict(str(integration.path / "services.yaml"))
except FileNotFoundError:
# Find if integration uses services
has_services = grep_dir(
@ -122,7 +122,7 @@ def validate_services(config: Config, integration: Integration) -> None:
)
return
except HomeAssistantError:
integration.add_error("services", "Unable to load services.yaml")
integration.add_error("services", "Invalid services.yaml")
return
try:

View File

@ -1102,7 +1102,7 @@ async def test_reload_automation_when_blueprint_changes(
autospec=True,
return_value=config,
), patch(
"homeassistant.components.blueprint.models.yaml.load_yaml",
"homeassistant.components.blueprint.models.yaml.load_yaml_dict",
autospec=True,
return_value=blueprint_config,
):

View File

@ -44,7 +44,7 @@ async def mock_yaml_dashboard(hass):
)
with patch(
"homeassistant.components.lovelace.dashboard.load_yaml",
"homeassistant.components.lovelace.dashboard.load_yaml_dict",
return_value={
"title": "YAML Title",
"views": [

View File

@ -141,7 +141,7 @@ async def test_lovelace_from_yaml(
events = async_capture_events(hass, const.EVENT_LOVELACE_UPDATED)
with patch(
"homeassistant.components.lovelace.dashboard.load_yaml",
"homeassistant.components.lovelace.dashboard.load_yaml_dict",
return_value={"hello": "yo"},
):
await client.send_json({"id": 7, "type": "lovelace/config"})
@ -154,7 +154,7 @@ async def test_lovelace_from_yaml(
# Fake new data to see we fire event
with patch(
"homeassistant.components.lovelace.dashboard.load_yaml",
"homeassistant.components.lovelace.dashboard.load_yaml_dict",
return_value={"hello": "yo2"},
):
await client.send_json({"id": 8, "type": "lovelace/config", "force": True})
@ -245,7 +245,7 @@ async def test_dashboard_from_yaml(
events = async_capture_events(hass, const.EVENT_LOVELACE_UPDATED)
with patch(
"homeassistant.components.lovelace.dashboard.load_yaml",
"homeassistant.components.lovelace.dashboard.load_yaml_dict",
return_value={"hello": "yo"},
):
await client.send_json(
@ -260,7 +260,7 @@ async def test_dashboard_from_yaml(
# Fake new data to see we fire event
with patch(
"homeassistant.components.lovelace.dashboard.load_yaml",
"homeassistant.components.lovelace.dashboard.load_yaml_dict",
return_value={"hello": "yo2"},
):
await client.send_json(

View File

@ -38,7 +38,7 @@ async def test_yaml_resources_backwards(
) -> None:
"""Test defining resources in YAML ll config (legacy)."""
with patch(
"homeassistant.components.lovelace.dashboard.load_yaml",
"homeassistant.components.lovelace.dashboard.load_yaml_dict",
return_value={"resources": RESOURCE_EXAMPLES},
):
assert await async_setup_component(

View File

@ -39,7 +39,7 @@ async def test_system_health_info_yaml(hass: HomeAssistant) -> None:
assert await async_setup_component(hass, "lovelace", {"lovelace": {"mode": "YAML"}})
await hass.async_block_till_done()
with patch(
"homeassistant.components.lovelace.dashboard.load_yaml",
"homeassistant.components.lovelace.dashboard.load_yaml_dict",
return_value={"views": [{"cards": []}]},
):
info = await get_system_health_info(hass, "lovelace")

View File

@ -57,7 +57,7 @@ async def test_turn_on_trigger_device_id(
assert calls[0].data["some"] == device.id
assert calls[0].data["id"] == 0
with patch("homeassistant.config.load_yaml", return_value={}):
with patch("homeassistant.config.load_yaml_dict", return_value={}):
await hass.services.async_call(automation.DOMAIN, SERVICE_RELOAD, blocking=True)
calls.clear()

View File

@ -60,7 +60,7 @@ async def test_webostv_turn_on_trigger_device_id(
assert calls[0].data["some"] == device.id
assert calls[0].data["id"] == 0
with patch("homeassistant.config.load_yaml", return_value={}):
with patch("homeassistant.config.load_yaml_dict", return_value={}):
await hass.services.async_call(automation.DOMAIN, SERVICE_RELOAD, blocking=True)
calls.clear()

View File

@ -272,7 +272,7 @@ async def test_zwave_js_value_updated(
clear_events()
with patch("homeassistant.config.load_yaml", return_value={}):
with patch("homeassistant.config.load_yaml_dict", return_value={}):
await hass.services.async_call(automation.DOMAIN, SERVICE_RELOAD, blocking=True)
@ -834,7 +834,7 @@ async def test_zwave_js_event(
clear_events()
with patch("homeassistant.config.load_yaml", return_value={}):
with patch("homeassistant.config.load_yaml_dict", return_value={}):
await hass.services.async_call(automation.DOMAIN, SERVICE_RELOAD, blocking=True)

View File

@ -134,6 +134,7 @@ def test_include_yaml(
[
({"/test/one.yaml": "one", "/test/two.yaml": "two"}, ["one", "two"]),
({"/test/one.yaml": "1", "/test/two.yaml": "2"}, [1, 2]),
({"/test/one.yaml": "1", "/test/two.yaml": None}, [1]),
],
)
def test_include_dir_list(
@ -190,6 +191,10 @@ def test_include_dir_list_recursive(
{"/test/first.yaml": "1", "/test/second.yaml": "2"},
{"first": 1, "second": 2},
),
(
{"/test/first.yaml": "1", "/test/second.yaml": None},
{"first": 1},
),
],
)
def test_include_dir_named(
@ -249,6 +254,10 @@ def test_include_dir_named_recursive(
{"/test/first.yaml": "- 1", "/test/second.yaml": "- 2\n- 3"},
[1, 2, 3],
),
(
{"/test/first.yaml": "- 1", "/test/second.yaml": None},
[1],
),
],
)
def test_include_dir_merge_list(
@ -311,6 +320,13 @@ def test_include_dir_merge_list_recursive(
},
{"key1": 1, "key2": 2, "key3": 3},
),
(
{
"/test/first.yaml": "key1: 1",
"/test/second.yaml": None,
},
{"key1": 1},
),
],
)
def test_include_dir_merge_named(
@ -686,3 +702,20 @@ def test_string_used_as_vol_schema(try_both_loaders) -> None:
schema({"key_1": "value_1", "key_2": "value_2"})
with pytest.raises(vol.Invalid):
schema({"key_1": "value_2", "key_2": "value_1"})
@pytest.mark.parametrize(
("hass_config_yaml", "expected_data"), [("", {}), ("bla:", {"bla": None})]
)
def test_load_yaml_dict(
try_both_loaders, mock_hass_config_yaml: None, expected_data: Any
) -> None:
"""Test item without a key."""
assert yaml.load_yaml_dict(YAML_CONFIG_FILE) == expected_data
@pytest.mark.parametrize("hass_config_yaml", ["abc", "123", "[]"])
def test_load_yaml_dict_fail(try_both_loaders, mock_hass_config_yaml: None) -> None:
"""Test item without a key."""
with pytest.raises(yaml_loader.YamlTypeError):
yaml_loader.load_yaml_dict(YAML_CONFIG_FILE)