From 7af0b5a6ccdee03a4e3a31918e9286b10f2357cc Mon Sep 17 00:00:00 2001 From: Joshua Date: Sat, 26 Sep 2026 07:18:49 -0400 Subject: [PATCH 1/3] Namespace quest definition icon keys with legacy back-compat Add core/icons.py as the one place icon keys are validated and normalized. A definition icon is now stored as lucide:, fa: or emoji: (at most 8 code points, no whitespace, control characters or markup). Create and edit validate strictly and store the canonical form; a legacy bare name is stored as lucide:, and None still clears the icon. Reads normalize leniently so existing bare-name rows need no data migration: the admin quest-definition response and the panel snapshot's per-instance icon both return the namespaced form. Task 707994b7. Co-Authored-By: Claude Opus 5.5 --- api/nestquest_core.py | 5 + api/routes_admin.py | 3 +- custom_components/nestquest/core/icons.py | 112 ++++++++++++++++++ .../nestquest/core/quest_definitions.py | 5 +- custom_components/nestquest/core/snapshot.py | 7 +- tests/test_api_admin_questdef.py | 90 +++++++++++++- tests/test_api_admin_questdef_list.py | 2 +- tests/test_core_icons.py | 105 ++++++++++++++++ tests/test_quest_definitions.py | 97 +++++++++++++-- 9 files changed, 411 insertions(+), 15 deletions(-) create mode 100644 custom_components/nestquest/core/icons.py create mode 100644 tests/test_core_icons.py diff --git a/api/nestquest_core.py b/api/nestquest_core.py index cb4ff1e..35100b6 100644 --- a/api/nestquest_core.py +++ b/api/nestquest_core.py @@ -152,6 +152,10 @@ core_quest_definitions = importlib.import_module( f"{_CORE_MODULE_NAME}.quest_definitions" ) +#: The icon-key module (``core.icons.normalize_icon_for_read``) the +#: admin plane's quest-definition responses normalize ``icon`` through. +core_icons = importlib.import_module(f"{_CORE_MODULE_NAME}.icons") + #: The recurrence model (``core.recurrence.ScheduleRule`` and #: ``RuleValidationError``) the quest-definition routes build request #: rules with β€” the model validates at construction, the routes never @@ -215,6 +219,7 @@ __all__ = [ "core_db", "core_events", "core_history", + "core_icons", "core_materialize", "core_migrations", "core_presence", diff --git a/api/routes_admin.py b/api/routes_admin.py index b70e8eb..23a8f1d 100644 --- a/api/routes_admin.py +++ b/api/routes_admin.py @@ -336,6 +336,7 @@ from api.nestquest_core import ( core_dao_children, core_dao_rules, core_history, + core_icons, core_materialize, core_presence_management, core_quest_definitions, @@ -1205,7 +1206,7 @@ def _quest_definition_response( id=bundle.definition.id, title=bundle.definition.title, description=bundle.definition.description, - icon=bundle.definition.icon, + icon=core_icons.normalize_icon_for_read(bundle.definition.icon), is_active=bundle.definition.is_active, skip_on_away=bundle.definition.skip_on_away, rule=_rule_response(bundle.rule), diff --git a/custom_components/nestquest/core/icons.py b/custom_components/nestquest/core/icons.py new file mode 100644 index 0000000..01bb14b --- /dev/null +++ b/custom_components/nestquest/core/icons.py @@ -0,0 +1,112 @@ +"""Namespaced quest-definition icon keys. + +A definition's ``icon`` is stored as ``:`` where the +namespace is one of :data:`ICON_NAMESPACES`: + +- ``lucide:`` / ``fa:`` β€” ```` is a lowercase kebab + name (ASCII ``[a-z0-9-]``, not starting or ending with ``-``). +- ``emoji:`` β€” a short emoji string of at most + :data:`EMOJI_MAX_CODE_POINTS` code points, with no whitespace, no + control characters and none of ``<``, ``>``, ``&``. + +Rows written before namespacing hold a bare Lucide name (``dog``). +:func:`validate_icon` (writes) normalizes such a legacy value to +``lucide:``; :func:`normalize_icon_for_read` (reads) does the same +leniently so legacy rows render without a data migration. + +Nothing here imports :mod:`homeassistant`. +""" +from __future__ import annotations + +import re +import unicodedata + +ICON_NAMESPACE_LUCIDE = "lucide" +ICON_NAMESPACE_FA = "fa" +ICON_NAMESPACE_EMOJI = "emoji" +ICON_NAMESPACES = frozenset( + {ICON_NAMESPACE_LUCIDE, ICON_NAMESPACE_FA, ICON_NAMESPACE_EMOJI} +) + +#: Upper bound on an emoji payload's length in code points β€” enough for +#: ZWJ sequences such as a family emoji, too short for arbitrary text. +EMOJI_MAX_CODE_POINTS = 8 + +_KEBAB_NAME = re.compile(r"[a-z0-9](?:[a-z0-9-]*[a-z0-9])?") +_EMOJI_FORBIDDEN = frozenset("<>&") + + +def _is_kebab_name(value: str) -> bool: + return _KEBAB_NAME.fullmatch(value) is not None + + +def _is_valid_emoji_payload(value: str) -> bool: + if not value or len(value) > EMOJI_MAX_CODE_POINTS: + return False + return not any( + char.isspace() + or char in _EMOJI_FORBIDDEN + or unicodedata.category(char) == "Cc" + for char in value + ) + + +def validate_icon(value: object) -> str | None: + """Return the canonical namespaced icon key, or raise naming ``icon``. + + ``None`` or an empty/whitespace-only string returns ``None`` (the + icon is cleared). A value with no ``:`` is a legacy bare Lucide + name and is stored as ``lucide:``. An unknown namespace, an + empty or malformed name, or an invalid emoji payload raises + ValueError. + """ + if value is None: + return None + if not isinstance(value, str): + raise ValueError(f"icon must be a string, got {value!r}") + trimmed = value.strip() + if not trimmed: + return None + + namespace, separator, name = trimmed.partition(":") + if not separator: + if not _is_kebab_name(trimmed): + raise ValueError( + f"icon must be a lowercase kebab name, got {value!r}" + ) + return f"{ICON_NAMESPACE_LUCIDE}:{trimmed}" + + if namespace not in ICON_NAMESPACES: + raise ValueError( + f"icon has an unknown namespace {namespace!r}; expected one of " + f"{sorted(ICON_NAMESPACES)}" + ) + if namespace == ICON_NAMESPACE_EMOJI: + if not _is_valid_emoji_payload(name): + raise ValueError( + f"icon emoji must be 1-{EMOJI_MAX_CODE_POINTS} code points " + f"with no whitespace, control characters or markup, " + f"got {value!r}" + ) + elif not _is_kebab_name(name): + raise ValueError( + f"icon {namespace} name must be a lowercase kebab name, " + f"got {value!r}" + ) + return trimmed + + +def normalize_icon_for_read(value: str | None) -> str | None: + """Return a stored icon in namespaced form for output. + + Lenient: ``None``/empty returns ``None``, a recognised namespace is + returned unchanged, and anything else becomes ``lucide:`` so + legacy rows render (an unknown name falls through to the frontend's + fallback icon rather than erroring). + """ + if not value or not value.strip(): + return None + namespace, separator, _ = value.partition(":") + if separator and namespace in ICON_NAMESPACES: + return value + return f"{ICON_NAMESPACE_LUCIDE}:{value}" diff --git a/custom_components/nestquest/core/quest_definitions.py b/custom_components/nestquest/core/quest_definitions.py index 8078ce2..58903eb 100644 --- a/custom_components/nestquest/core/quest_definitions.py +++ b/custom_components/nestquest/core/quest_definitions.py @@ -59,6 +59,7 @@ from .dao_rules import ( schedule_rule_to_storage, ) from .db import NestQuestDatabase +from .icons import validate_icon from .materialize import regenerate_for_definition from .recurrence import ScheduleRule, occurs_on @@ -248,7 +249,7 @@ async def create_quest_definition( assignee_ids = _validate_assignee_ids(assignee_child_ids) window_specs = _normalize_windows(windows) description_value = _validate_text(description, "description") - icon_value = _validate_text(icon, "icon") + icon_value = validate_icon(icon) skip_on_away_value = _validate_skip_on_away(skip_on_away) storage = schedule_rule_to_storage(rule) @@ -351,7 +352,7 @@ async def edit_quest_definition( icon_value: str | None | object = _UNSET if icon is not _UNSET: - icon_value = _validate_text(icon, "icon") + icon_value = validate_icon(icon) rule_storage: ScheduleRuleStorage | object = _UNSET if rule is not _UNSET: diff --git a/custom_components/nestquest/core/snapshot.py b/custom_components/nestquest/core/snapshot.py index 662a2f9..88679be 100644 --- a/custom_components/nestquest/core/snapshot.py +++ b/custom_components/nestquest/core/snapshot.py @@ -42,6 +42,7 @@ from .dao_instances import CompletionEventsDao, QuestInstancesDao from .dao_presence import PresenceOverridesDao, PresencePatternsDao from .dao_rules import QuestDefinitionsDao from .db import NestQuestDatabase +from .icons import normalize_icon_for_read from .presence import ( MAX_PREVIEW_SCAN_DAYS, PresenceEngine, @@ -262,7 +263,11 @@ async def build_snapshot( definition_id=instance.definition_id, child_id=instance.child_id, title=definition.title if definition else "Quest", - icon=definition.icon if definition else None, + icon=( + normalize_icon_for_read(definition.icon) + if definition + else None + ), window=instance.window, due_date=instance.due_date, due_time=instance.due_time, diff --git a/tests/test_api_admin_questdef.py b/tests/test_api_admin_questdef.py index 3a0f398..8850b43 100644 --- a/tests/test_api_admin_questdef.py +++ b/tests/test_api_admin_questdef.py @@ -35,6 +35,7 @@ for malformed bodies. """ from __future__ import annotations +import datetime import importlib from types import SimpleNamespace @@ -223,7 +224,7 @@ async def test_create_persists_definition_with_assignees_and_windows( } assert payload["title"] == "Tidy the den" assert payload["description"] == "Put everything back where it belongs." - assert payload["icon"] == "toy-box" + assert payload["icon"] == "lucide:toy-box" assert payload["is_active"] is True assert payload["skip_on_away"] is True assert payload["rule"] == _weekly_rule_dict() @@ -245,7 +246,7 @@ async def test_create_persists_definition_with_assignees_and_windows( assert stored.definition.description == ( "Put everything back where it belongs." ) - assert stored.definition.icon == "toy-box" + assert stored.definition.icon == "lucide:toy-box" assert stored.definition.is_active is True assert stored.rule.to_dict() == _weekly_rule_dict() assert sorted(child.id for child in stored.assignees) == [ada, bo] @@ -396,7 +397,7 @@ async def test_edit_persists_supplied_fields_and_leaves_omitted_untouched( assert stored.definition.description == ( "Put everything back where it belongs." ) - assert stored.definition.icon == "toy-box" + assert stored.definition.icon == "lucide:toy-box" assert stored.rule.to_dict() == _weekly_rule_dict() # assignee_child_ids was omitted: the assignees are untouched. assert sorted(child.id for child in stored.assignees) == [ada, bo] @@ -468,6 +469,89 @@ async def test_edit_can_clear_description_and_icon_explicitly( assert stored.rule.to_dict() == _weekly_rule_dict() +async def test_legacy_bare_icon_reads_back_namespaced( + admin_questdefs: SimpleNamespace, +) -> None: + """A row stored with a pre-namespacing bare icon reads as ``lucide:``. + + The row is rewritten through the DAO (bypassing the write-side + validation, as a row from before namespacing would be) and then read + back through BOTH consumers: the admin definition response and the + kids' panel snapshot's per-instance ``icon``. + """ + client = admin_questdefs.client + ada = (await _create_children(client, ("Ada",)))[0] + today = datetime.date.today().isoformat() + created = await client.post( + "/api/v1/admin/quest-definitions", + json={ + "title": "Walk the dog", + "rule": {"rule_type": "daily", "start_date": today}, + "assignee_child_ids": [ada], + "windows": ["evening"], + "icon": "fa:dog", + }, + headers=_admin_headers(), + ) + assert created.status_code == 201 + definition_id = created.json()["id"] + + await _dao_rules.QuestDefinitionsDao( + admin_questdefs.database + ).edit_definition(definition_id, icon="dog") + stored = (await _definitions_by_id(admin_questdefs.database))[ + definition_id + ] + assert stored.definition.icon == "dog" + + listed = await client.get( + "/api/v1/admin/quest-definitions", headers=_admin_headers() + ) + assert listed.status_code == 200 + [definition] = [ + d for d in listed.json()["definitions"] if d["id"] == definition_id + ] + assert definition["icon"] == "lucide:dog" + + panel = await client.get( + "/api/v1/panel/snapshot", + headers={"Authorization": "Bearer admin-test-panel-token"}, + ) + assert panel.status_code == 200 + [child] = panel.json()["children"] + icons = { + instance["icon"] + for instance in child["instances"] + if instance["definition_id"] == definition_id + } + assert icons == {"lucide:dog"} + + +@pytest.mark.parametrize( + "icon", ["foo:dog", "lucide:", "emoji:", "javascript:alert(1)"] +) +async def test_create_invalid_icon_is_422_naming_icon( + admin_questdefs: SimpleNamespace, icon: str +) -> None: + """An invalid icon key is a 422 whose detail names the icon field.""" + client = admin_questdefs.client + ada = (await _create_children(client, ("Ada",)))[0] + response = await client.post( + "/api/v1/admin/quest-definitions", + json={ + "title": "Walk the dog", + "rule": _weekly_rule_body(), + "assignee_child_ids": [ada], + "windows": ["morning"], + "icon": icon, + }, + headers=_admin_headers(), + ) + assert response.status_code == 422 + assert "icon" in str(response.json()["detail"]) + assert await _definitions_by_id(admin_questdefs.database) == {} + + async def test_edit_unknown_definition_is_404( admin_questdefs: SimpleNamespace, ) -> None: diff --git a/tests/test_api_admin_questdef_list.py b/tests/test_api_admin_questdef_list.py index 0584695..d9afb63 100644 --- a/tests/test_api_admin_questdef_list.py +++ b/tests/test_api_admin_questdef_list.py @@ -135,7 +135,7 @@ async def test_admin_lists_active_and_inactive_definitions_with_full_shape( "id": active_id, "title": "Tidy the den", "description": "Put everything back where it belongs.", - "icon": "toy-box", + "icon": "lucide:toy-box", "is_active": True, "skip_on_away": True, "rule": { diff --git a/tests/test_core_icons.py b/tests/test_core_icons.py new file mode 100644 index 0000000..f4a7bf2 --- /dev/null +++ b/tests/test_core_icons.py @@ -0,0 +1,105 @@ +"""Test the namespaced icon-key module (task 707994b7). + +:func:`validate_icon` is the strict write-side check (create/edit); +:func:`normalize_icon_for_read` the lenient read-side normalizer that +lets legacy bare Lucide names render without a data migration. +""" +from __future__ import annotations + +import pytest + +from custom_components.nestquest.core.icons import ( + EMOJI_MAX_CODE_POINTS, + ICON_NAMESPACES, + normalize_icon_for_read, + validate_icon, +) + + +def test_namespace_and_cap_constants() -> None: + assert ICON_NAMESPACES == {"lucide", "fa", "emoji"} + assert EMOJI_MAX_CODE_POINTS == 8 + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + ("lucide:dog", "lucide:dog"), + ("fa:dog", "fa:dog"), + ("fa:paw-print", "fa:paw-print"), + ("lucide:toy-box-2", "lucide:toy-box-2"), + ("emoji:πŸ•", "emoji:πŸ•"), + # A ZWJ family sequence (7 code points) is within the cap. + ("emoji:πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦", "emoji:πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦"), + ("emoji:" + "⭐" * EMOJI_MAX_CODE_POINTS, "emoji:" + "⭐" * 8), + # Legacy bare Lucide names normalize to the lucide namespace. + ("dog", "lucide:dog"), + (" toy-box ", "lucide:toy-box"), + (" fa:dog ", "fa:dog"), + ], +) +def test_validate_icon_accepts(value: str, expected: str) -> None: + assert validate_icon(value) == expected + + +@pytest.mark.parametrize("value", [None, "", " ", "\t\n"]) +def test_validate_icon_clears(value: str | None) -> None: + assert validate_icon(value) is None + + +@pytest.mark.parametrize( + "value", + [ + "foo:dog", # unknown namespace + "javascript:alert(1)", # unknown namespace (and markup-ish) + "mdi:dog", # not one of the supported namespaces + "lucide:", # empty name + "fa:", # empty name + "emoji:", # empty payload + "lucide:Dog", # uppercase + "lucide:has space", # whitespace + "lucide:-dog", # leading hyphen + "lucide:dog-", # trailing hyphen + "lucide:dog_bone", # not kebab + "fa:dΓΆg", # non-ASCII + "lucide:a:b", # colon inside the name + "Dog", # bare legacy name must still be kebab + "has space", + "