Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support in blueprint schema for input sections #110513

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
+129 −23
Open

Support in blueprint schema for input sections #110513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
945a55b
initial commit for sections
karwosts Feb 13, 2024
e1e0605
updates
karwosts Feb 14, 2024
5fe2888
add description
karwosts Feb 14, 2024
2c9d1a0
fix test
karwosts Feb 14, 2024
7cca7a1
rename collapsed key
karwosts Mar 1, 2024
fd7dd77
Merge branch 'home-assistant:dev' into blueprint-sections
karwosts Mar 29, 2024
703f445
New schema
karwosts Mar 29, 2024
ef031cd
update snapshots
karwosts Mar 30, 2024
58f68d9
Testing for sections
karwosts Mar 30, 2024
c06074c
Validate no duplicate input keys across sections
karwosts Apr 7, 2024
312a274
rename all_inputs
karwosts Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions homeassistant/components/blueprint/const.py
View file
Open in desktop
Expand Up @@ -9,5 +9,6 @@
CONF_HOMEASSISTANT = "homeassistant"
CONF_MIN_VERSION = "min_version"
CONF_AUTHOR = "author"
CONF_COLLAPSED = "collapsed"

DOMAIN = "blueprint"
11 changes: 9 additions & 2 deletions homeassistant/components/blueprint/models.py
View file
Open in desktop
Expand Up @@ -78,7 +78,7 @@ def __init__(

self.domain = data_domain

missing = yaml.extract_inputs(data) - set(data[CONF_BLUEPRINT][CONF_INPUT])
missing = yaml.extract_inputs(data) - set(self.inputs)

if missing:
raise InvalidBlueprint(
Expand All @@ -96,7 +96,14 @@ def name(self) -> str:
@property
def inputs(self) -> dict[str, Any]:
"""Return blueprint inputs."""
return self.data[CONF_BLUEPRINT][CONF_INPUT] # type: ignore[no-any-return]
inputs = {}
for key, value in self.data[CONF_BLUEPRINT][CONF_INPUT].items():
if value and CONF_INPUT in value:
for key, value in value[CONF_INPUT].items():
inputs[key] = value
else:
inputs[key] = value
return inputs
Comment on lines +99 to +106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The requests was to do the flattening in frontend, now the flattening happens in core instead if I understand it correctly. Will this have any side effect on existing blueprinted automations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reread the comment thread but I never saw a request to "do the flattering in frontend", or at least I missed that if it was implied.

I'm not quite sure what that would really mean anyway, don't I need to have a single list of inputs for core to iterate them for replacement?

I'm also not sure what the concern is here for backward compatibility, for BPs without sections (all of them today), isn't this block of code just basically a no-op?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flattening in frontend is what @bramkragten meant here: #110513 (comment) unless I misunderstand.
However, the solution you've come with seems fine too, I just want another pair of eyes on it.


@property
def metadata(self) -> dict[str, Any]:
Expand Down
50 changes: 44 additions & 6 deletions homeassistant/components/blueprint/schemas.py
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@
CONF_DEFAULT,
CONF_DESCRIPTION,
CONF_DOMAIN,
CONF_ICON,
CONF_NAME,
CONF_PATH,
CONF_SELECTOR,
Expand All @@ -18,6 +19,7 @@
from .const import (
CONF_AUTHOR,
CONF_BLUEPRINT,
CONF_COLLAPSED,
CONF_HOMEASSISTANT,
CONF_INPUT,
CONF_MIN_VERSION,
Expand Down Expand Up @@ -46,6 +48,23 @@ def version_validator(value: Any) -> str:
return value


def unique_input_validator(inputs: Any) -> Any:
"""Validate the inputs don't have duplicate keys under different sections."""
all_inputs = set()
for key, value in inputs.items():
if value and CONF_INPUT in value:
for key in value[CONF_INPUT]:
if key in all_inputs:
raise vol.Invalid(f"Duplicate use of input key {key} in blueprint.")
all_inputs.add(key)
else:
if key in all_inputs:
raise vol.Invalid(f"Duplicate use of input key {key} in blueprint.")
all_inputs.add(key)

return inputs


@callback
def is_blueprint_config(config: Any) -> bool:
"""Return if it is a blueprint config."""
Expand All @@ -67,6 +86,21 @@ def is_blueprint_instance_config(config: Any) -> bool:
}
)

BLUEPRINT_INPUT_SECTION_SCHEMA = vol.Schema(
{
vol.Optional(CONF_NAME): str,
vol.Optional(CONF_ICON): str,
vol.Optional(CONF_DESCRIPTION): str,
vol.Optional(CONF_COLLAPSED): bool,
vol.Required(CONF_INPUT, default=dict): {
str: vol.Any(
None,
BLUEPRINT_INPUT_SCHEMA,
)
},
}
)

BLUEPRINT_SCHEMA = vol.Schema(
emontnemery marked this conversation as resolved.
Show resolved Hide resolved
{
vol.Required(CONF_BLUEPRINT): vol.Schema(
Expand All @@ -79,12 +113,16 @@ def is_blueprint_instance_config(config: Any) -> bool:
vol.Optional(CONF_HOMEASSISTANT): {
vol.Optional(CONF_MIN_VERSION): version_validator
},
vol.Optional(CONF_INPUT, default=dict): {
str: vol.Any(
None,
BLUEPRINT_INPUT_SCHEMA,
)
},
vol.Optional(CONF_INPUT, default=dict): vol.All(
{
str: vol.Any(
None,
BLUEPRINT_INPUT_SCHEMA,
BLUEPRINT_INPUT_SECTION_SCHEMA,
)
},
unique_input_validator,
),
}
),
},
Expand Down
6 changes: 3 additions & 3 deletions tests/components/blueprint/snapshots/test_importer.ambr
View file
Open in desktop
@@ -1,6 +1,6 @@
# serializer version: 1
# name: test_extract_blueprint_from_community_topic
NodeDictClass({
dict({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe what is happening here is the inputs used to come from the yaml loader, so they were the special class NodeDictClass, but now the inputs are put together as a dictionary manually in the inputs function, as it has to merge them together from several sections, so that's why the snapshot need to be updated.

'brightness': NodeDictClass({
'default': 50,
'description': 'Brightness of the light(s) when turning on',
Expand Down Expand Up @@ -97,7 +97,7 @@
})
# ---
# name: test_fetch_blueprint_from_community_url
NodeDictClass({
dict({
'brightness': NodeDictClass({
'default': 50,
'description': 'Brightness of the light(s) when turning on',
Expand Down Expand Up @@ -194,7 +194,7 @@
})
# ---
# name: test_fetch_blueprint_from_github_gist_url
NodeDictClass({
dict({
'light_entity': NodeDictClass({
'name': 'Light',
'selector': dict({
Expand Down
38 changes: 26 additions & 12 deletions tests/components/blueprint/test_models.py
View file
Open in desktop
Expand Up @@ -26,24 +26,38 @@ def blueprint_1():
)


@pytest.fixture
def blueprint_2():
@pytest.fixture(params=[False, True])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm doing here (I think) is for every test that uses blueprint_2, I'm creating an additional variant that uses the same blueprint, except that the inputs are put into sections.

This shouldn't functionally affect the test, so the test code should still pass equally whether the inputs are in sections or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but for the final version of the PR, please add a new blueprint_3 or blueprint_2_sectioned instead of branching in the fixture.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I make an entirely new fixture, is it still possible to reuse the existing tests that currently consume blueprint_2? That's why I used the param so I can leverage and don't have to duplicate the tests.

I'm not so familiar with pytest so the concept of how to run a test with multiple different fixtures I haven't grasped yet.

def blueprint_2(request):
"""Blueprint fixture with default inputs."""
return models.Blueprint(
{
"blueprint": {
"name": "Hello",
"domain": "automation",
"source_url": "https://github.com/balloob/home-assistant-config/blob/main/blueprints/automation/motion_light.yaml",
blueprint = {
"blueprint": {
"name": "Hello",
"domain": "automation",
"source_url": "https://github.com/balloob/home-assistant-config/blob/main/blueprints/automation/motion_light.yaml",
"input": {
"test-input": {"name": "Name", "description": "Description"},
"test-input-default": {"default": "test"},
},
},
"example": Input("test-input"),
"example-default": Input("test-input-default"),
}
if request.param:
# Replace the inputs with inputs in sections. Test should otherwise behave the same.
blueprint["blueprint"]["input"] = {
"section-1": {
"name": "Section 1",
"input": {
"test-input": {"name": "Name", "description": "Description"},
"test-input-default": {"default": "test"},
},
},
"example": Input("test-input"),
"example-default": Input("test-input-default"),
"section-2": {
"input": {
"test-input-default": {"default": "test"},
}
},
}
)
return models.Blueprint(blueprint)


@pytest.fixture
Expand Down
46 changes: 46 additions & 0 deletions tests/components/blueprint/test_schemas.py
View file
Open in desktop
Expand Up @@ -52,6 +52,24 @@
},
}
},
# With input sections
{
"blueprint": {
"name": "Test Name",
"domain": "automation",
"input": {
"section_a": {
"input": {"some_placeholder": None},
},
"section_b": {
"name": "Section",
"description": "A section with no inputs",
"input": {},
},
"some_placeholder_2": None,
},
}
},
],
)
def test_blueprint_schema(blueprint) -> None:
Expand Down Expand Up @@ -94,6 +112,34 @@ def test_blueprint_schema(blueprint) -> None:
},
}
},
# Duplicate inputs in sections (1 of 2)
{
"blueprint": {
"name": "Test Name",
"domain": "automation",
"input": {
"section_a": {
"input": {"some_placeholder": None},
},
"section_b": {
"input": {"some_placeholder": None},
},
},
}
},
# Duplicate inputs in sections (2 of 2)
{
"blueprint": {
"name": "Test Name",
"domain": "automation",
"input": {
"section_a": {
"input": {"some_placeholder": None},
},
"some_placeholder": None,
},
}
},
],
)
def test_blueprint_schema_invalid(blueprint) -> None:
Expand Down