diff --git a/frigate/api/app.py b/frigate/api/app.py index 49b555606d..7f78f4b56c 100644 --- a/frigate/api/app.py +++ b/frigate/api/app.py @@ -31,6 +31,7 @@ from frigate.api.auth import ( get_allowed_cameras_for_filter, require_role, ) +from frigate.api.config_util import swap_runtime_config from frigate.api.defs.query.app_query_parameters import AppTimelineHourlyQueryParameters from frigate.api.defs.request.app_body import ( AppConfigSetBody, @@ -915,19 +916,7 @@ def config_set(request: Request, body: AppConfigSetBody): if body.requires_restart == 0 or body.update_topic: old_config: FrigateConfig = request.app.frigate_config - request.app.frigate_config = config - request.app.genai_manager.update_config(config) - - if request.app.profile_manager is not None: - request.app.profile_manager.update_config(config) - - if request.app.stats_emitter is not None: - request.app.stats_emitter.config = config - - if request.app.dispatcher is not None: - request.app.dispatcher.config = config - for comm in request.app.dispatcher.comms: - comm.config = config + swap_runtime_config(request.app, config) if body.update_topic: if body.update_topic.startswith("config/cameras/"): diff --git a/frigate/api/camera.py b/frigate/api/camera.py index a86c35883f..29e861b3f5 100644 --- a/frigate/api/camera.py +++ b/frigate/api/camera.py @@ -25,6 +25,7 @@ from frigate.api.auth import ( require_go2rtc_stream_access, require_role, ) +from frigate.api.config_util import swap_runtime_config from frigate.api.defs.request.app_body import CameraSetBody from frigate.api.defs.tags import Tags from frigate.config import FrigateConfig @@ -1254,9 +1255,14 @@ async def delete_camera( status_code=500, ) - # Update runtime config - request.app.frigate_config = config - request.app.genai_manager.update_config(config) + # rebind every collaborator to the new config and re-layer runtime + # toggles for the surviving cameras, same as /api/config/set + swap_runtime_config(request.app, config) + + # drop the deleted camera's persisted overrides so a camera later + # added under the same name doesn't inherit them + if request.app.dispatcher is not None: + request.app.dispatcher.clear_runtime_state_for_camera(camera_name) # Publish removal to stop ffmpeg processes and clean up runtime state request.app.config_publisher.publish_update( diff --git a/frigate/api/config_util.py b/frigate/api/config_util.py new file mode 100644 index 0000000000..6a95a4ab01 --- /dev/null +++ b/frigate/api/config_util.py @@ -0,0 +1,33 @@ +"""Shared helpers for applying a freshly parsed config to the running app.""" + +from fastapi import FastAPI + +from frigate.config import FrigateConfig + + +def swap_runtime_config(app: FastAPI, config: FrigateConfig) -> None: + """Point every long-lived collaborator at a newly parsed config object. + + Both /api/config/set and camera deletion re-parse yaml into a fresh + FrigateConfig and must rebind the same set of references, or the API and + the dispatcher drift onto different objects (the API reports one camera + state while the dispatcher acts on another). Runtime toggle overrides are + re-layered last: the swap rebuilt every camera from yaml, so without this a + camera the user turned off would silently come back on. + """ + app.frigate_config = config + app.genai_manager.update_config(config) + + if app.profile_manager is not None: + app.profile_manager.update_config(config) + + if app.stats_emitter is not None: + app.stats_emitter.config = config + + if app.dispatcher is not None: + app.dispatcher.config = config + + for comm in app.dispatcher.comms: + comm.config = config + + app.dispatcher.apply_runtime_state() diff --git a/frigate/comms/dispatcher.py b/frigate/comms/dispatcher.py index 6cb4f21b07..29f5fd97bc 100644 --- a/frigate/comms/dispatcher.py +++ b/frigate/comms/dispatcher.py @@ -404,38 +404,64 @@ class Dispatcher: for comm in self.comms: comm.stop() - def restore_runtime_state(self) -> None: + def apply_runtime_state(self) -> dict[str, dict[str, bool]]: """Replay persisted runtime overrides through the camera settings handlers. - Called once after Frigate startup completes so processing threads can - receive the resulting ``config_updater`` broadcasts. Unknown cameras - and topics are skipped; handler exceptions are logged and replay - continues for remaining entries. + Routing through the handlers (rather than mutating config directly) is + deliberate: they publish the ``config_updater`` broadcast and the + retained MQTT state as a side effect, so worker processes and the UI + converge on the replayed value. Unknown cameras and topics are skipped; + handler exceptions are logged and replay continues for the rest. + + Returns: + The entries handed to a handler without raising, keyed by camera + then topic. A handler can still refuse the value internally (an ON + payload for a camera that is not enabled_in_config, for example), + so this is not proof the override took effect. """ state = self._runtime_state.load() + applied: dict[str, dict[str, bool]] = {} + for camera_name, features in state.items(): if camera_name not in self.config.cameras: continue + for topic, value in features.items(): handler = self._camera_settings_handlers.get(topic) + if handler is None: continue + payload = "ON" if value else "OFF" + try: handler(camera_name, payload) except Exception: logger.exception( - "Failed to restore runtime state %s.%s=%s", + "Failed to apply runtime state %s.%s=%s", camera_name, topic, payload, ) continue + + applied.setdefault(camera_name, {})[topic] = value + + return applied + + def restore_runtime_state(self) -> None: + """Replay persisted runtime overrides once Frigate startup completes. + + Called after every ``config_updater`` subscriber is up so the resulting + broadcasts are not dropped by ZMQ PUB/SUB. + """ + for camera_name, features in self.apply_runtime_state().items(): + for topic, value in features.items(): logger.info( "Restored runtime state: %s.%s=%s", camera_name, topic, - payload, + "ON" if value else "OFF", ) def clear_runtime_state_for_yaml_keys(self, dotted_keys: Iterable[str]) -> None: @@ -458,6 +484,14 @@ class Dispatcher: """ self._runtime_state.clear_all() + def clear_runtime_state_for_camera(self, camera: str) -> None: + """Drop all persisted runtime overrides for a deleted camera. + + Called by camera deletion so a camera later added under the same name + does not inherit the removed camera's stale toggles. + """ + self._runtime_state.clear_camera(camera) + def _on_detect_command(self, camera_name: str, payload: str) -> None: """Callback for detect topic.""" detect_settings = self.config.cameras[camera_name].detect diff --git a/frigate/comms/runtime_state.py b/frigate/comms/runtime_state.py index 222d488ec9..be1b850e92 100644 --- a/frigate/comms/runtime_state.py +++ b/frigate/comms/runtime_state.py @@ -96,6 +96,25 @@ class RuntimeStatePersistence: except OSError: logger.exception("Failed to clear runtime state") + def clear_camera(self, camera: str) -> None: + """Drop every stored override for a single camera. + + Called when a camera is deleted so a camera later added under the same + name does not inherit the removed camera's stale toggles. + """ + try: + with FileLock(self._lock_path, timeout=self._lock_timeout): + data = self._read_locked() + cameras = data.get("cameras") + if not isinstance(cameras, dict) or camera not in cameras: + return + del cameras[camera] + self._write_locked(data) + except Timeout: + logger.error("Timed out clearing runtime state for camera") + except OSError: + logger.exception("Failed to clear runtime state for camera") + def clear_for_yaml_keys(self, dotted_keys: Iterable[str]) -> None: """Remove stored entries whose YAML key was just rewritten. diff --git a/frigate/config/profile_manager.py b/frigate/config/profile_manager.py index cfd0dc9df5..2aafbab57d 100644 --- a/frigate/config/profile_manager.py +++ b/frigate/config/profile_manager.py @@ -141,6 +141,11 @@ class ProfileManager: Preserves active profile state: re-snapshots base configs from the new (freshly parsed) config, then re-applies profile overrides if a profile was active. + + Deliberately does not clear the dispatcher's runtime overrides. This is + the config-save path, not a profile switch: the save only invalidates + the toggles it rewrote in yaml, which /api/config/set already clears by + key. The broad wipe belongs to activate_profile alone. """ current_active = self.config.active_profile self.config = new_config @@ -164,10 +169,6 @@ class ProfileManager: self.config.active_profile = None self._persist_active_profile(None) - # drop all runtime overrides so they don't replay stale values on restart - if self.dispatcher is not None: - self.dispatcher.clear_runtime_state() - def activate_profile( self, profile_name: str | None, diff --git a/frigate/test/http_api/test_http_camera.py b/frigate/test/http_api/test_http_camera.py new file mode 100644 index 0000000000..afeac1bde2 --- /dev/null +++ b/frigate/test/http_api/test_http_camera.py @@ -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() diff --git a/frigate/test/http_api/test_http_config_set.py b/frigate/test/http_api/test_http_config_set.py index 48b1ac2c76..cd249c6fa6 100644 --- a/frigate/test/http_api/test_http_config_set.py +++ b/frigate/test/http_api/test_http_config_set.py @@ -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() diff --git a/frigate/test/test_config_util.py b/frigate/test/test_config_util.py new file mode 100644 index 0000000000..310b233aaa --- /dev/null +++ b/frigate/test/test_config_util.py @@ -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() diff --git a/frigate/test/test_dispatcher_runtime_state.py b/frigate/test/test_dispatcher_runtime_state.py index dae0518d80..e0a91536cb 100644 --- a/frigate/test/test_dispatcher_runtime_state.py +++ b/frigate/test/test_dispatcher_runtime_state.py @@ -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() diff --git a/frigate/test/test_profiles.py b/frigate/test/test_profiles.py index 59dc077466..e2f9a0bed9 100644 --- a/frigate/test/test_profiles.py +++ b/frigate/test/test_profiles.py @@ -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): diff --git a/frigate/test/test_runtime_state.py b/frigate/test/test_runtime_state.py index 6143184030..5a373ade9e 100644 --- a/frigate/test/test_runtime_state.py +++ b/frigate/test/test_runtime_state.py @@ -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()