mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-06-16 09:38:38 +00:00
fix(web): preserve dotted schema keys when saving plugin config (#370)
The plugin config form posts form-data with dot-notation paths (e.g. "leagues.fifa.world.enabled"). _get_schema_property and _set_nested_value split those paths on every dot, so a schema key that itself contains a dot (soccer league keys like "fifa.world", "eng.1") was mistaken for nested "fifa" -> "world" objects. Per-league edits (enable, favorite_teams, nested booleans) were written to a fabricated "leagues.fifa.world" branch while the real league object was never updated, so saves silently dropped the change and produced a byte-identical config. Both helpers now greedily match the longest path segment that exists in the schema (_get_schema_property) or the config being updated (_set_nested_value), mirroring the frontend's dotted-key handling. Adds regression tests covering schema lookup, value typing, and writes under dotted league keys, plus a guard that plain nested paths still work. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
93
test/web_interface/test_dotted_league_keys.py
Normal file
93
test/web_interface/test_dotted_league_keys.py
Normal file
@@ -0,0 +1,93 @@
|
|||||||
|
"""
|
||||||
|
Regression test for saving plugin config fields whose schema keys contain dots
|
||||||
|
(e.g. soccer league keys like "fifa.world", "eng.1", "usa.1").
|
||||||
|
|
||||||
|
Bug: the web config form posts form-data with dotted paths such as
|
||||||
|
"leagues.fifa.world.enabled". The helpers that resolve those paths split on every
|
||||||
|
dot, so the dotted league key "fifa.world" was mistaken for nested "fifa" ->
|
||||||
|
"world" objects. Per-league edits (enable, favorite_teams, nested booleans) were
|
||||||
|
written to a fabricated "leagues.fifa.world" branch while the real league object
|
||||||
|
was never updated, so the save silently dropped the change and the saved config
|
||||||
|
came out byte-identical.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
from web_interface.blueprints.api_v3 import (
|
||||||
|
_get_schema_property,
|
||||||
|
_set_nested_value,
|
||||||
|
_parse_form_value_with_schema,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
SCHEMA = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"leagues": {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"fifa.world": {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"enabled": {"type": "boolean"},
|
||||||
|
"favorite_teams": {
|
||||||
|
"type": "array",
|
||||||
|
"items": {"type": "string"},
|
||||||
|
},
|
||||||
|
"display_modes": {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"live": {"type": "boolean"}},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
class TestDottedLeagueKeys(unittest.TestCase):
|
||||||
|
def test_schema_lookup_resolves_dotted_league_key(self):
|
||||||
|
prop = _get_schema_property(SCHEMA, "leagues.fifa.world.favorite_teams")
|
||||||
|
self.assertIsNotNone(prop, "dotted league key path should resolve")
|
||||||
|
self.assertEqual(prop.get("type"), "array")
|
||||||
|
|
||||||
|
def test_schema_lookup_resolves_nested_object_beneath_dotted_key(self):
|
||||||
|
live = _get_schema_property(SCHEMA, "leagues.fifa.world.display_modes.live")
|
||||||
|
self.assertIsNotNone(live)
|
||||||
|
self.assertEqual(live.get("type"), "boolean")
|
||||||
|
|
||||||
|
def test_parse_typed_value_for_dotted_key(self):
|
||||||
|
# Comma-separated text input "USA" must become an array, not the raw string.
|
||||||
|
parsed = _parse_form_value_with_schema(
|
||||||
|
"USA", "leagues.fifa.world.favorite_teams", SCHEMA
|
||||||
|
)
|
||||||
|
self.assertEqual(parsed, ["USA"])
|
||||||
|
|
||||||
|
def test_set_value_updates_real_league_not_fabricated_branch(self):
|
||||||
|
config = {"leagues": {"fifa.world": {"enabled": False, "favorite_teams": []}}}
|
||||||
|
_set_nested_value(config, "leagues.fifa.world.enabled", True)
|
||||||
|
_set_nested_value(config, "leagues.fifa.world.favorite_teams", ["USA"])
|
||||||
|
|
||||||
|
self.assertTrue(config["leagues"]["fifa.world"]["enabled"])
|
||||||
|
self.assertEqual(config["leagues"]["fifa.world"]["favorite_teams"], ["USA"])
|
||||||
|
# The real league must be updated and no fabricated "fifa" branch created.
|
||||||
|
self.assertNotIn("fifa", config["leagues"])
|
||||||
|
|
||||||
|
def test_set_value_into_missing_leaf_lands_in_real_league(self):
|
||||||
|
# A leaf that does not exist yet still resolves into the real dotted league.
|
||||||
|
config = {"leagues": {"fifa.world": {"enabled": False}}}
|
||||||
|
_set_nested_value(config, "leagues.fifa.world.display_modes.live", True)
|
||||||
|
self.assertTrue(
|
||||||
|
config["leagues"]["fifa.world"]["display_modes"]["live"]
|
||||||
|
)
|
||||||
|
self.assertNotIn("fifa", config["leagues"])
|
||||||
|
|
||||||
|
def test_plain_nested_paths_still_work(self):
|
||||||
|
config = {}
|
||||||
|
_set_nested_value(config, "customization.text.font", "small")
|
||||||
|
self.assertEqual(config["customization"]["text"]["font"], "small")
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -3558,21 +3558,29 @@ def _get_schema_property(schema, key_path):
|
|||||||
|
|
||||||
parts = key_path.split('.')
|
parts = key_path.split('.')
|
||||||
current = schema['properties']
|
current = schema['properties']
|
||||||
|
i = 0
|
||||||
|
|
||||||
for i, part in enumerate(parts):
|
while i < len(parts):
|
||||||
if part not in current:
|
# Try progressively longer candidates, longest first, so schema keys that
|
||||||
return None
|
# themselves contain dots (e.g. league keys like "fifa.world") are matched
|
||||||
|
# instead of being mistaken for nested "fifa" -> "world" objects.
|
||||||
prop = current[part]
|
matched = False
|
||||||
|
for j in range(len(parts), i, -1):
|
||||||
# If this is the last part, return the property
|
candidate = '.'.join(parts[i:j])
|
||||||
if i == len(parts) - 1:
|
if isinstance(current, dict) and candidate in current:
|
||||||
return prop
|
prop = current[candidate]
|
||||||
|
# Consumed all remaining parts — this is the target property.
|
||||||
# If this is an object with properties, navigate deeper
|
if j == len(parts):
|
||||||
if isinstance(prop, dict) and 'properties' in prop:
|
return prop
|
||||||
current = prop['properties']
|
# Navigate deeper through an object with properties.
|
||||||
else:
|
if isinstance(prop, dict) and 'properties' in prop:
|
||||||
|
current = prop['properties']
|
||||||
|
i = j
|
||||||
|
matched = True
|
||||||
|
break
|
||||||
|
# Matched a non-object before consuming the path — can't go deeper.
|
||||||
|
return None
|
||||||
|
if not matched:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
return None
|
return None
|
||||||
@@ -3745,10 +3753,45 @@ def _parse_form_value_with_schema(value, key_path, schema):
|
|||||||
return value
|
return value
|
||||||
|
|
||||||
|
|
||||||
|
def _resolve_key_segments(key_path, config):
|
||||||
|
"""Split a dot-notation path into segments, greedily preserving keys that
|
||||||
|
themselves contain dots (e.g. league keys like "fifa.world").
|
||||||
|
|
||||||
|
At each level the longest candidate that matches a key already present in the
|
||||||
|
config wins; otherwise the path splits on the next dot (the normal
|
||||||
|
nested-create case). Because dotted keys such as ``leagues."fifa.world"``
|
||||||
|
always exist in the saved config being updated, this routes the value to the
|
||||||
|
real league object instead of fabricating a ``leagues.fifa.world`` tree.
|
||||||
|
"""
|
||||||
|
parts = key_path.split('.')
|
||||||
|
segments = []
|
||||||
|
node = config
|
||||||
|
i = 0
|
||||||
|
while i < len(parts):
|
||||||
|
matched = False
|
||||||
|
if isinstance(node, dict):
|
||||||
|
for j in range(len(parts), i, -1):
|
||||||
|
candidate = '.'.join(parts[i:j])
|
||||||
|
if candidate in node:
|
||||||
|
segments.append(candidate)
|
||||||
|
node = node[candidate]
|
||||||
|
i = j
|
||||||
|
matched = True
|
||||||
|
break
|
||||||
|
if not matched:
|
||||||
|
part = parts[i]
|
||||||
|
segments.append(part)
|
||||||
|
node = node.get(part) if isinstance(node, dict) else None
|
||||||
|
i += 1
|
||||||
|
return segments
|
||||||
|
|
||||||
|
|
||||||
def _set_nested_value(config, key_path, value):
|
def _set_nested_value(config, key_path, value):
|
||||||
"""
|
"""
|
||||||
Set a value in a nested dict using dot notation path.
|
Set a value in a nested dict using dot notation path.
|
||||||
Handles existing nested dicts correctly by merging instead of replacing.
|
Handles existing nested dicts correctly by merging instead of replacing.
|
||||||
|
Keys containing dots (e.g. league keys like "fifa.world") are preserved when
|
||||||
|
they already exist in the config rather than being split into nested objects.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
config: The config dict to modify
|
config: The config dict to modify
|
||||||
@@ -3758,22 +3801,22 @@ def _set_nested_value(config, key_path, value):
|
|||||||
# Skip setting if value is the sentinel
|
# Skip setting if value is the sentinel
|
||||||
if value is _SKIP_FIELD:
|
if value is _SKIP_FIELD:
|
||||||
return
|
return
|
||||||
|
|
||||||
parts = key_path.split('.')
|
segments = _resolve_key_segments(key_path, config)
|
||||||
current = config
|
current = config
|
||||||
|
|
||||||
# Navigate/create intermediate dicts
|
# Navigate/create intermediate dicts
|
||||||
for i, part in enumerate(parts[:-1]):
|
for seg in segments[:-1]:
|
||||||
if part not in current:
|
if seg not in current:
|
||||||
current[part] = {}
|
current[seg] = {}
|
||||||
elif not isinstance(current[part], dict):
|
elif not isinstance(current[seg], dict):
|
||||||
# If the existing value is not a dict, replace it with a dict
|
# If the existing value is not a dict, replace it with a dict
|
||||||
current[part] = {}
|
current[seg] = {}
|
||||||
current = current[part]
|
current = current[seg]
|
||||||
|
|
||||||
# Set the final value (don't overwrite with empty dict if value is None and we want to preserve structure)
|
# Set the final value (don't overwrite with empty dict if value is None and we want to preserve structure)
|
||||||
if value is not None or parts[-1] not in current:
|
if value is not None or segments[-1] not in current:
|
||||||
current[parts[-1]] = value
|
current[segments[-1]] = value
|
||||||
|
|
||||||
|
|
||||||
def _set_missing_booleans_to_false(config, schema_props, form_keys, prefix='', config_node=None):
|
def _set_missing_booleans_to_false(config, schema_props, form_keys, prefix='', config_node=None):
|
||||||
|
|||||||
Reference in New Issue
Block a user