fix config corruption when deleting the last commented mask or zone

Deleting the last entry of a mapping or sequence via config/set orphaned ruamel's comment tokens, which were then emitted above a flow-style {} / [] at column 0, which is unparseable yaml that failed validation and silently rolled the change back. The fix is to clear the emptied collection's stale comment metadata (and the parent's entry for it) so the dump stays valid. Non-empty collections are left untouched so sibling comments are preserved. This covers both emptied maps and emptied lists.
This commit is contained in:
Josh Hawkins 2026-07-04 07:50:00 -05:00
parent 729ee86043
commit feddf4f869
2 changed files with 208 additions and 0 deletions

View File

@ -0,0 +1,183 @@
"""Test in-place yaml config updates."""
import os
import tempfile
import unittest
from ruamel.yaml import YAML
from frigate.util.builtin import update_yaml_file_bulk
class TestUpdateYaml(unittest.TestCase):
def setUp(self) -> None:
self.yaml = YAML()
fd, self.config_path = tempfile.mkstemp(suffix=".yml")
os.close(fd)
def tearDown(self) -> None:
os.unlink(self.config_path)
def _write(self, text: str) -> None:
with open(self.config_path, "w") as f:
f.write(text)
def _read(self) -> str:
with open(self.config_path) as f:
return f.read()
def _load(self):
with open(self.config_path) as f:
return self.yaml.load(f)
def test_delete_key(self):
"""Deleting a key removes it and leaves valid yaml."""
self._write(
"cameras:\n"
" cam1:\n"
" objects:\n"
" filters:\n"
" car:\n"
" mask: 0,0.45,0.245,0.45\n"
)
update_yaml_file_bulk(
self.config_path, {"cameras.cam1.objects.filters.car.mask": ""}
)
data = self._load()
assert "mask" not in data["cameras"]["cam1"]["objects"]["filters"]["car"]
def test_delete_commented_key_emptying_map(self):
"""Deleting the only key of a map whose key carries comments must not
emit unparseable yaml (orphaned comment tokens above a flow-style {})."""
self._write(
"cameras:\n"
" cam1:\n"
" objects:\n"
" filters:\n"
" car:\n"
" # cars parked across the street\n"
" # second comment line\n"
" mask: 0,0.45,0.245,0.45\n"
" motion:\n"
" mask: 0,0.449,0.686,0.395\n"
)
update_yaml_file_bulk(
self.config_path, {"cameras.cam1.objects.filters.car.mask": ""}
)
# must re-parse cleanly
data = self._load()
assert "mask" not in data["cameras"]["cam1"]["objects"]["filters"]["car"]
assert data["cameras"]["cam1"]["motion"]["mask"] == "0,0.449,0.686,0.395"
# the orphaned comments must be gone from the file, not just parseable
content = self._read()
assert "cars parked across the street" not in content
assert "second comment line" not in content
def test_delete_last_named_mask_emptying_map(self):
"""The path the current UI actually sends: a named object mask deleted
down to an empty `mask` map, with a comment inside that map."""
self._write(
"cameras:\n"
" cam1:\n"
" objects:\n"
" filters:\n"
" car:\n"
" mask:\n"
" # ignore the neighbor's driveway\n"
" driveway:\n"
" coordinates: 0,0.1,0.2,0.3\n"
)
update_yaml_file_bulk(
self.config_path,
{"cameras.cam1.objects.filters.car.mask.driveway": ""},
)
data = self._load()
assert data["cameras"]["cam1"]["objects"]["filters"]["car"]["mask"] == {}
assert "ignore the neighbor's driveway" not in self._read()
def test_delete_last_commented_list_item(self):
"""Deleting the last element of a commented sequence must not emit
an orphaned comment above a flow-style [] at column 0."""
self._write(
"cameras:\n"
" cam1:\n"
" motion:\n"
" mask:\n"
" # driveway motion mask\n"
" - 0,0.4,0.6,0.4\n"
)
update_yaml_file_bulk(self.config_path, {"cameras.cam1.motion.mask.0": ""})
data = self._load()
assert data["cameras"]["cam1"]["motion"]["mask"] == []
assert "driveway motion mask" not in self._read()
def test_delete_list_item_preserves_remaining(self):
"""Deleting one element of a sequence keeps the others and stays valid."""
self._write(
"cameras:\n"
" cam1:\n"
" motion:\n"
" mask:\n"
" - 0,0.4,0.6,0.4\n"
" - 0,0.1,0.2,0.3\n"
)
update_yaml_file_bulk(self.config_path, {"cameras.cam1.motion.mask.0": ""})
data = self._load()
assert data["cameras"]["cam1"]["motion"]["mask"] == ["0,0.1,0.2,0.3"]
def test_delete_key_preserves_siblings(self):
"""Deleting one key among several keeps the sibling entries and any
comments on keys preceding the deleted one."""
self._write(
"cameras:\n"
" cam1:\n"
" objects:\n"
" filters:\n"
" car:\n"
" # mask drawn around the parked suv\n"
" mask: 0,0.45,0.245,0.45\n"
" threshold: 0.8\n"
)
update_yaml_file_bulk(
self.config_path, {"cameras.cam1.objects.filters.car.threshold": ""}
)
data = self._load()
car = data["cameras"]["cam1"]["objects"]["filters"]["car"]
assert "threshold" not in car
assert car["mask"] == "0,0.45,0.245,0.45"
assert "# mask drawn around the parked suv" in self._read()
def test_delete_first_commented_key_keeps_map_valid(self):
"""Deleting a commented key from a map that still has other keys
leaves the remaining entries intact and the file parseable."""
self._write(
"cameras:\n"
" cam1:\n"
" objects:\n"
" filters:\n"
" car:\n"
" # comment on the deleted key\n"
" mask: 0,0.45,0.245,0.45\n"
" threshold: 0.8\n"
)
update_yaml_file_bulk(
self.config_path, {"cameras.cam1.objects.filters.car.mask": ""}
)
data = self._load()
car = data["cameras"]["cam1"]["objects"]["filters"]["car"]
assert "mask" not in car
assert car["threshold"] == 0.8
def test_update_value_preserves_comments(self):
"""Updating a value keeps surrounding comments intact."""
self._write(
"cameras:\n cam1:\n detect:\n # tuned for the pi\n fps: 4\n"
)
update_yaml_file_bulk(self.config_path, {"cameras.cam1.detect.fps": 5})
data = self._load()
assert data["cameras"]["cam1"]["detect"]["fps"] == 5
assert "# tuned for the pi" in self._read()
if __name__ == "__main__":
unittest.main(verbosity=2)

View File

@ -293,26 +293,51 @@ def update_yaml_file_bulk(file_path: str, updates: Dict[str, Any]):
logger.error(f"Unable to write to Frigate config file {file_path}: {e}")
def clear_orphaned_comments(collection, parent, parent_key) -> None:
"""Drop stale ruamel comment tokens after a deletion empties a collection.
When the last entry of a mapping or sequence is removed, any comments that
lived inside that collection's block are orphaned. ruamel then emits them
above a flow-style `{}`/`[]` dedented to column 0, which is unparseable and
corrupts the config. Clearing the emptied collection's own comment metadata
(and the parent's entry pointing at it) keeps the dump valid. Non-empty
collections are left untouched so comments on remaining siblings survive.
"""
if not hasattr(collection, "ca") or len(collection) != 0:
return
collection.ca.items.clear()
collection.ca.comment = None
if parent is not None and hasattr(parent, "ca"):
parent.ca.items.pop(parent_key, None)
def update_yaml(data, key_path, new_value):
temp = data
parent = None
parent_key = None
for key in key_path[:-1]:
if isinstance(key, tuple):
if key[0] not in temp:
temp[key[0]] = [{}] * max(1, key[1] + 1)
elif len(temp[key[0]]) <= key[1]:
temp[key[0]] += [{}] * (key[1] - len(temp[key[0]]) + 1)
parent, parent_key = temp[key[0]], key[1]
temp = temp[key[0]][key[1]]
else:
if key not in temp or temp[key] is None:
temp[key] = {}
parent, parent_key = temp, key
temp = temp[key]
last_key = key_path[-1]
if new_value == "":
if isinstance(last_key, tuple):
del temp[last_key[0]][last_key[1]]
clear_orphaned_comments(temp[last_key[0]], temp, last_key[0])
else:
del temp[last_key]
clear_orphaned_comments(temp, parent, parent_key)
else:
if isinstance(last_key, tuple):
if last_key[0] not in temp: