mirror of
https://github.com/blakeblackshear/frigate.git
synced 2026-08-02 17:12:16 +03:00
Fix persisted runtime camera toggles (#23734)
* preserve runtime camera toggles across config saves Runtime toggles (camera on/off, detect, recordings, snapshots, audio) mutate the in-memory config and persist an override to .runtime_state.json. /api/config/set re-parses yaml into a fresh FrigateConfig and swaps it in, re-applying the yaml and profile layers but dropping the runtime layer, so a camera turned off from the dashboard came back on when an unrelated camera was saved. The workers were never notified, so it only appeared to come back: the UI streamed go2rtc while ffmpeg stayed stopped. Extract the startup replay into Dispatcher.apply_runtime_state() and call it from config_set after the swap, re-layering the overrides and republishing them so workers and the UI reconverge. Remove the broad clear_runtime_state() from ProfileManager.update_config, which is only ever reached from config_set: with a profile active, every save wiped every camera's overrides from disk. The broad wipe stays in activate_profile, where a real profile switch does invalidate the steady state. Saves still clear the keys they rewrote via clear_runtime_state_for_yaml_keys, so yaml wins where the two disagree. * sync runtime config on camera delete and prune its overrides Deleting a camera re-parsed yaml into a fresh FrigateConfig but only rebound app.frigate_config and genai_manager, never dispatcher.config (nor profile_manager, stats_emitter, or the runtime overrides). The API and the dispatcher then drifted onto different config objects until the next config save re-synced them, so the API reported surviving cameras with their yaml enabled state while the dispatcher still acted on their real runtime state. Extract the config swap that config_set already does into a shared swap_runtime_config helper and call it from both sites, so every collaborator is rebound and the surviving cameras' runtime toggles are re-layered. Also drop the deleted camera's persisted overrides via a new clear_camera so a camera later added under the same name does not inherit them.
This commit is contained in:
Josh Hawkins
@@ -0,0 +1,132 @@
|
||||
"""Tests for the camera delete endpoint's runtime config handling."""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import MagicMock, Mock, patch
|
||||
|
||||
import ruamel.yaml
|
||||
|
||||
from frigate.config import FrigateConfig
|
||||
from frigate.config.camera.updater import CameraConfigUpdatePublisher
|
||||
from frigate.models import Event, Recordings, ReviewSegment
|
||||
from frigate.test.http_api.base_http_test import AuthTestClient, BaseTestHttp
|
||||
|
||||
|
||||
class TestDeleteCameraRuntimeConfig(BaseTestHttp):
|
||||
"""Deleting a camera must keep the API and dispatcher on the same config."""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp(models=[Event, Recordings, ReviewSegment])
|
||||
self.minimal_config = {
|
||||
"mqtt": {"host": "mqtt"},
|
||||
"cameras": {
|
||||
"front_door": {
|
||||
"ffmpeg": {
|
||||
"inputs": [
|
||||
{"path": "rtsp://10.0.0.1:554/video", "roles": ["detect"]}
|
||||
]
|
||||
},
|
||||
"detect": {"height": 1080, "width": 1920, "fps": 5},
|
||||
},
|
||||
"back_yard": {
|
||||
"ffmpeg": {
|
||||
"inputs": [
|
||||
{"path": "rtsp://10.0.0.2:554/video", "roles": ["detect"]}
|
||||
]
|
||||
},
|
||||
"detect": {"height": 720, "width": 1280, "fps": 10},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
def _write_config_file(self):
|
||||
yaml = ruamel.yaml.YAML()
|
||||
f = tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False)
|
||||
yaml.dump(self.minimal_config, f)
|
||||
f.close()
|
||||
return f.name
|
||||
|
||||
def _create_app_with_dispatcher(self, dispatcher):
|
||||
from fastapi import Request
|
||||
|
||||
from frigate.api.auth import get_allowed_cameras_for_filter, get_current_user
|
||||
from frigate.api.fastapi_app import create_fastapi_app
|
||||
|
||||
mock_publisher = Mock(spec=CameraConfigUpdatePublisher)
|
||||
mock_publisher.publisher = MagicMock()
|
||||
|
||||
app = create_fastapi_app(
|
||||
FrigateConfig(**self.minimal_config),
|
||||
self.db,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
mock_publisher,
|
||||
None,
|
||||
dispatcher=dispatcher,
|
||||
enforce_default_admin=False,
|
||||
)
|
||||
|
||||
async def mock_get_current_user(request: Request):
|
||||
return {
|
||||
"username": request.headers.get("remote-user"),
|
||||
"role": request.headers.get("remote-role"),
|
||||
}
|
||||
|
||||
async def mock_get_allowed_cameras_for_filter(request: Request):
|
||||
return list(self.minimal_config.get("cameras", {}).keys())
|
||||
|
||||
app.dependency_overrides[get_current_user] = mock_get_current_user
|
||||
app.dependency_overrides[get_allowed_cameras_for_filter] = (
|
||||
mock_get_allowed_cameras_for_filter
|
||||
)
|
||||
|
||||
return app, mock_publisher
|
||||
|
||||
@patch("frigate.api.camera.requests.delete")
|
||||
@patch("frigate.api.camera.cleanup_camera_files")
|
||||
@patch("frigate.api.camera.cleanup_camera_db")
|
||||
@patch("frigate.api.camera.find_config_file")
|
||||
def test_delete_syncs_dispatcher_and_prunes_runtime_state(
|
||||
self, mock_find_config, mock_cleanup_db, mock_cleanup_files, mock_go2rtc_delete
|
||||
):
|
||||
"""Deleting a camera swaps every config reference and prunes its state."""
|
||||
config_path = self._write_config_file()
|
||||
mock_find_config.return_value = config_path
|
||||
mock_cleanup_db.return_value = ({}, [])
|
||||
|
||||
dispatcher = MagicMock()
|
||||
dispatcher.comms = []
|
||||
|
||||
try:
|
||||
app, _ = self._create_app_with_dispatcher(dispatcher)
|
||||
|
||||
with AuthTestClient(app) as client:
|
||||
resp = client.delete("/cameras/front_door")
|
||||
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
self.assertTrue(resp.json()["success"])
|
||||
|
||||
# the dispatcher must be moved onto the same new object the API
|
||||
# now serves, and that object must no longer contain the camera
|
||||
self.assertIs(dispatcher.config, app.frigate_config)
|
||||
self.assertNotIn("front_door", dispatcher.config.cameras)
|
||||
self.assertIn("back_yard", dispatcher.config.cameras)
|
||||
|
||||
# surviving cameras' overrides are re-layered onto the new object
|
||||
dispatcher.apply_runtime_state.assert_called_once_with()
|
||||
|
||||
# the deleted camera's persisted overrides are pruned
|
||||
dispatcher.clear_runtime_state_for_camera.assert_called_once_with(
|
||||
"front_door"
|
||||
)
|
||||
finally:
|
||||
os.unlink(config_path)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -91,6 +91,124 @@ class TestConfigSetWildcardPropagation(BaseTestHttp):
|
||||
|
||||
return app, mock_publisher
|
||||
|
||||
def _create_app_with_dispatcher(self, dispatcher):
|
||||
"""Create app with a mocked config publisher and a real-ish dispatcher."""
|
||||
from fastapi import Request
|
||||
|
||||
from frigate.api.auth import get_allowed_cameras_for_filter, get_current_user
|
||||
from frigate.api.fastapi_app import create_fastapi_app
|
||||
|
||||
mock_publisher = Mock(spec=CameraConfigUpdatePublisher)
|
||||
mock_publisher.publisher = MagicMock()
|
||||
|
||||
app = create_fastapi_app(
|
||||
FrigateConfig(**self.minimal_config),
|
||||
self.db,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
mock_publisher,
|
||||
None,
|
||||
dispatcher=dispatcher,
|
||||
enforce_default_admin=False,
|
||||
)
|
||||
|
||||
async def mock_get_current_user(request: Request):
|
||||
username = request.headers.get("remote-user")
|
||||
role = request.headers.get("remote-role")
|
||||
return {"username": username, "role": role}
|
||||
|
||||
async def mock_get_allowed_cameras_for_filter(request: Request):
|
||||
return list(self.minimal_config.get("cameras", {}).keys())
|
||||
|
||||
app.dependency_overrides[get_current_user] = mock_get_current_user
|
||||
app.dependency_overrides[get_allowed_cameras_for_filter] = (
|
||||
mock_get_allowed_cameras_for_filter
|
||||
)
|
||||
|
||||
return app, mock_publisher
|
||||
|
||||
@patch("frigate.api.app.find_config_file")
|
||||
def test_runtime_disabled_camera_survives_unrelated_save(self, mock_find_config):
|
||||
"""A camera turned off at runtime stays off when another camera is saved."""
|
||||
config_path = self._write_config_file()
|
||||
mock_find_config.return_value = config_path
|
||||
|
||||
dispatcher = MagicMock()
|
||||
dispatcher.comms = []
|
||||
|
||||
# front_door was turned off via the UI: the override is on disk, and
|
||||
# yaml still says enabled: true. Stand in for the real replay, which
|
||||
# reads dispatcher.config - the object the endpoint just swapped in.
|
||||
def fake_apply():
|
||||
dispatcher.config.cameras["front_door"].enabled = False
|
||||
return {"front_door": {"enabled": False}}
|
||||
|
||||
dispatcher.apply_runtime_state.side_effect = fake_apply
|
||||
|
||||
try:
|
||||
app, _ = self._create_app_with_dispatcher(dispatcher)
|
||||
|
||||
with AuthTestClient(app) as client:
|
||||
resp = client.put(
|
||||
"/config/set",
|
||||
json={
|
||||
"config_data": {
|
||||
"cameras": {"back_yard": {"detect": {"fps": 7}}}
|
||||
},
|
||||
"requires_restart": 0,
|
||||
},
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
self.assertTrue(resp.json()["success"])
|
||||
|
||||
# the swap must be repaired: the new config object the API and
|
||||
# dispatcher now share has to still show front_door as off
|
||||
dispatcher.apply_runtime_state.assert_called_once_with()
|
||||
self.assertFalse(app.frigate_config.cameras["front_door"].enabled)
|
||||
self.assertIs(dispatcher.config, app.frigate_config)
|
||||
|
||||
# yaml-wins ordering: the surgical clear for rewritten keys
|
||||
# must run before the replay, or a save that rewrote a toggle
|
||||
# would have its old override resurrected
|
||||
call_names = [name for name, _, _ in dispatcher.mock_calls]
|
||||
self.assertLess(
|
||||
call_names.index("clear_runtime_state_for_yaml_keys"),
|
||||
call_names.index("apply_runtime_state"),
|
||||
)
|
||||
finally:
|
||||
os.unlink(config_path)
|
||||
|
||||
@patch("frigate.api.app.find_config_file")
|
||||
def test_no_reapply_when_config_is_not_swapped(self, mock_find_config):
|
||||
"""A restart-required save with no update topic never swaps, so no replay."""
|
||||
config_path = self._write_config_file()
|
||||
mock_find_config.return_value = config_path
|
||||
|
||||
dispatcher = MagicMock()
|
||||
dispatcher.comms = []
|
||||
|
||||
try:
|
||||
app, _ = self._create_app_with_dispatcher(dispatcher)
|
||||
|
||||
with AuthTestClient(app) as client:
|
||||
resp = client.put(
|
||||
"/config/set",
|
||||
json={
|
||||
"config_data": {"mqtt": {"host": "other"}},
|
||||
"requires_restart": 1,
|
||||
},
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
dispatcher.apply_runtime_state.assert_not_called()
|
||||
finally:
|
||||
os.unlink(config_path)
|
||||
|
||||
def _write_config_file(self):
|
||||
"""Write the minimal config to a temp YAML file and return the path."""
|
||||
yaml = ruamel.yaml.YAML()
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
"""Tests for the shared runtime config swap helper."""
|
||||
|
||||
import unittest
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from frigate.api.config_util import swap_runtime_config
|
||||
|
||||
|
||||
class TestSwapRuntimeConfig(unittest.TestCase):
|
||||
"""swap_runtime_config rebinds every collaborator to the new config."""
|
||||
|
||||
def _make_app(self) -> MagicMock:
|
||||
app = MagicMock()
|
||||
app.dispatcher.comms = [MagicMock(), MagicMock()]
|
||||
return app
|
||||
|
||||
def test_rebinds_all_references(self) -> None:
|
||||
app = self._make_app()
|
||||
config = MagicMock(name="new_config")
|
||||
|
||||
swap_runtime_config(app, config)
|
||||
|
||||
self.assertIs(app.frigate_config, config)
|
||||
app.genai_manager.update_config.assert_called_once_with(config)
|
||||
app.profile_manager.update_config.assert_called_once_with(config)
|
||||
self.assertIs(app.stats_emitter.config, config)
|
||||
self.assertIs(app.dispatcher.config, config)
|
||||
for comm in app.dispatcher.comms:
|
||||
self.assertIs(comm.config, config)
|
||||
|
||||
def test_reapplies_runtime_state_after_swap(self) -> None:
|
||||
app = self._make_app()
|
||||
config = MagicMock(name="new_config")
|
||||
|
||||
swap_runtime_config(app, config)
|
||||
|
||||
# the swap rebuilds cameras from yaml, so overrides must be re-layered
|
||||
app.dispatcher.apply_runtime_state.assert_called_once_with()
|
||||
|
||||
def test_tolerates_missing_optional_collaborators(self) -> None:
|
||||
app = MagicMock()
|
||||
app.profile_manager = None
|
||||
app.stats_emitter = None
|
||||
app.dispatcher = None
|
||||
config = MagicMock(name="new_config")
|
||||
|
||||
# must not raise when the optional collaborators are absent
|
||||
swap_runtime_config(app, config)
|
||||
|
||||
self.assertIs(app.frigate_config, config)
|
||||
app.genai_manager.update_config.assert_called_once_with(config)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -126,6 +126,40 @@ class TestRestoreRuntimeState(unittest.TestCase):
|
||||
self.dispatcher.restore_runtime_state()
|
||||
self.handler_mocks["detect"].assert_called_once_with("front_door", "ON")
|
||||
|
||||
def test_apply_runtime_state_replays_through_handlers(self) -> None:
|
||||
"""The extracted method replays every stored entry."""
|
||||
with patch.object(
|
||||
self.dispatcher._runtime_state,
|
||||
"load",
|
||||
return_value={"front_door": {"enabled": False, "detect": True}},
|
||||
):
|
||||
self.dispatcher.apply_runtime_state()
|
||||
|
||||
self.handler_mocks["enabled"].assert_called_once_with("front_door", "OFF")
|
||||
self.handler_mocks["detect"].assert_called_once_with("front_door", "ON")
|
||||
|
||||
def test_apply_runtime_state_returns_applied_entries(self) -> None:
|
||||
"""Callers get back what was replayed, for logging and assertions."""
|
||||
with patch.object(
|
||||
self.dispatcher._runtime_state,
|
||||
"load",
|
||||
return_value={"front_door": {"enabled": False}, "nope": {"enabled": True}},
|
||||
):
|
||||
applied = self.dispatcher.apply_runtime_state()
|
||||
|
||||
self.assertEqual(applied, {"front_door": {"enabled": False}})
|
||||
|
||||
def test_restore_runtime_state_still_replays(self) -> None:
|
||||
"""The startup entry point keeps working after the extraction."""
|
||||
with patch.object(
|
||||
self.dispatcher._runtime_state,
|
||||
"load",
|
||||
return_value={"back_yard": {"snapshots": False}},
|
||||
):
|
||||
self.dispatcher.restore_runtime_state()
|
||||
|
||||
self.handler_mocks["snapshots"].assert_called_once_with("back_yard", "OFF")
|
||||
|
||||
|
||||
class TestHandlersPersistViaSet(unittest.TestCase):
|
||||
"""Verify each in-scope handler writes to the runtime state on success."""
|
||||
@@ -212,6 +246,12 @@ class TestClearPassthrough(unittest.TestCase):
|
||||
dispatcher.clear_runtime_state()
|
||||
dispatcher._runtime_state.clear_all.assert_called_once_with()
|
||||
|
||||
def test_clear_runtime_state_for_camera_passthrough(self) -> None:
|
||||
dispatcher = _build_dispatcher({})
|
||||
dispatcher._runtime_state = MagicMock(spec=RuntimeStatePersistence)
|
||||
dispatcher.clear_runtime_state_for_camera("front_door")
|
||||
dispatcher._runtime_state.clear_camera.assert_called_once_with("front_door")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -786,8 +786,15 @@ class TestProfileManager(unittest.TestCase):
|
||||
dispatcher.clear_runtime_state.assert_not_called()
|
||||
|
||||
@patch.object(ProfileManager, "_persist_active_profile")
|
||||
def test_update_config_clears_when_active_profile_reapplies(self, mock_persist):
|
||||
"""After /api/config/set, an active-profile re-application drops state."""
|
||||
def test_update_config_preserves_runtime_state_with_active_profile(
|
||||
self, mock_persist
|
||||
):
|
||||
"""A config/set save must not wipe overrides it never rewrote.
|
||||
|
||||
The save path clears matching entries itself via
|
||||
clear_runtime_state_for_yaml_keys; a broad wipe here would drop
|
||||
overrides for unrelated cameras.
|
||||
"""
|
||||
dispatcher = MagicMock()
|
||||
manager = ProfileManager(self.config, self.mock_updater, dispatcher)
|
||||
manager.activate_profile("armed")
|
||||
@@ -795,7 +802,20 @@ class TestProfileManager(unittest.TestCase):
|
||||
|
||||
new_config = FrigateConfig(**self.config_data)
|
||||
manager.update_config(new_config)
|
||||
dispatcher.clear_runtime_state.assert_called_once_with()
|
||||
dispatcher.clear_runtime_state.assert_not_called()
|
||||
|
||||
@patch.object(ProfileManager, "_persist_active_profile")
|
||||
def test_update_config_still_reapplies_active_profile(self, mock_persist):
|
||||
"""Dropping the wipe must not disturb profile re-application."""
|
||||
dispatcher = MagicMock()
|
||||
manager = ProfileManager(self.config, self.mock_updater, dispatcher)
|
||||
manager.activate_profile("armed")
|
||||
|
||||
new_config = FrigateConfig(**self.config_data)
|
||||
manager.update_config(new_config)
|
||||
|
||||
self.assertEqual(manager.config, new_config)
|
||||
self.assertEqual(new_config.active_profile, "armed")
|
||||
|
||||
@patch.object(ProfileManager, "_persist_active_profile")
|
||||
def test_update_config_does_not_clear_when_no_active_profile(self, mock_persist):
|
||||
|
||||
@@ -131,6 +131,25 @@ class TestRuntimeStatePersistence(unittest.TestCase):
|
||||
self.store.clear_all()
|
||||
self.assertEqual(self.store.load(), {})
|
||||
|
||||
def test_clear_camera_removes_only_that_camera(self) -> None:
|
||||
self.store.set("front_door", "enabled", False)
|
||||
self.store.set("front_door", "detect", False)
|
||||
self.store.set("back_yard", "audio", False)
|
||||
|
||||
self.store.clear_camera("front_door")
|
||||
|
||||
self.assertEqual(self.store.load(), {"back_yard": {"audio": False}})
|
||||
|
||||
def test_clear_camera_is_noop_for_unknown_camera(self) -> None:
|
||||
self.store.set("front_door", "enabled", False)
|
||||
self.store.clear_camera("side_gate")
|
||||
self.assertEqual(self.store.load(), {"front_door": {"enabled": False}})
|
||||
|
||||
def test_clear_camera_is_safe_when_file_missing(self) -> None:
|
||||
# No prior set() calls, so the file does not exist
|
||||
self.store.clear_camera("front_door")
|
||||
self.assertEqual(self.store.load(), {})
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user