diff --git a/behavision/cameras.py b/behavision/cameras.py index 289d428..78bbf5b 100644 --- a/behavision/cameras.py +++ b/behavision/cameras.py @@ -182,13 +182,22 @@ class CameraStore: After that the store is authoritative — otherwise a camera the user deleted in the UI would reappear on every restart. + + Entries with no address are dropped rather than imported. The bundled + config declares one whose host comes from the environment, which is + right for a checkout with a .env and resolves to nothing on every + machine the product is actually installed on. Seeding that would put a + camera the shop never added into a brand new install, permanently + failing to connect to "" — and the first thing they would have to do is + work out what it was and delete it. """ with self._lock: - if self.path.exists() or not cameras: + usable = [c for c in cameras if c.addressed()] + if self.path.exists() or not usable: return False - for cam in cameras: + for cam in usable: self._cameras[cam.id] = cam self._save() log.info("seeded %d camera(s) from YAML into %s", - len(cameras), self.path) + len(usable), self.path) return True diff --git a/behavision/config.py b/behavision/config.py index bb944b3..19b09be 100644 --- a/behavision/config.py +++ b/behavision/config.py @@ -55,6 +55,43 @@ class CameraConfig(BaseModel): max_width: int = 1280 # frames wider than this are downscaled at ingest tuning: CameraTuning = CameraTuning() + @model_validator(mode="before") + @classmethod + def _normalize_blanks(cls, values): + # Unset ${ENV} placeholders parse as YAML null, not "" - the same trap + # ApiSection and EmailSection already guard against, and this section + # did not. The bundled config/default.yaml reads its camera's host, + # username and password from the environment, so on any machine + # WITHOUT a .env - which is every machine the product is installed on - + # loading it raised three pydantic errors and the engine would not + # start at all. Found by cloning the repository and running the tests. + if not isinstance(values, dict): + return values + # Named fields only, never a blanket None -> "". `webcam` is an + # Optional[int] whose None is meaningful ("this is not a webcam"), and + # `tuning` is a nested model; sweeping either into "" trades one + # validation error for another. + values = dict(values) + for key in ("url", "host", "path", "username", "password"): + if values.get(key) is None: + values[key] = "" + for key, default in (("port", 554), ("max_width", 1280), ("path", "/")): + if values.get(key) in (None, ""): + values[key] = default + return values + + def addressed(self) -> bool: + """True when this entry actually names something to connect to. + + The bundled config declares a camera whose address comes from the + environment, for development. On an installed PC there is no such + environment, so that entry resolves to a camera with no address - and + seeding it would put a permanently-failing camera nobody added into + every fresh install, retrying a connection to "" forever. An entry with + no url, no host and no webcam is not a camera. + """ + return bool(self.url or self.host) or self.webcam is not None + def source(self) -> "str | int": """Resolved capture source: webcam index, explicit URL, or a URL built from parts with percent-encoded credentials.""" diff --git a/tests/test_config.py b/tests/test_config.py index 5144995..dee7807 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,6 +1,8 @@ """The shipped YAML carries thresholds measured on the deployment site; the pydantic defaults must not drift away from them (a trimmed config would then silently re-admit the false positives those values were tuned to reject).""" +from pathlib import Path + import pytest from behavision.config import (ApiSection, Config, DetectionSection, @@ -123,3 +125,62 @@ def test_no_usable_model_fails_loudly(tmp_path): (tmp_path / "w600k_mbf.onnx").write_bytes(b"garbage") with pytest.raises(FileNotFoundError, match="no usable recognition model"): ArcFaceEncoder(tmp_path) + + +def _no_env(monkeypatch, tmp_path): + """Make the process look like an installed PC rather than a checkout. + + `load_config` calls `load_dotenv` first, so simply unsetting the variables + is not enough in a working tree - the .env sitting next to the code puts + them straight back. Removing the .env from the picture is the condition + these tests are actually about. + """ + import behavision.paths as paths + monkeypatch.setattr(paths, "env_file", lambda: None) + for var in ("BEHAVISION_CAM1_HOST", "BEHAVISION_CAM1_USERNAME", + "BEHAVISION_CAM1_PASSWORD", "BEHAVISION_API_USER", + "BEHAVISION_API_PASSWORD", "BEHAVISION_WEBHOOK_URL"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setenv("BEHAVISION_DATA_DIR", str(tmp_path)) + + +def test_the_bundled_config_loads_with_no_environment_at_all(tmp_path, monkeypatch): + """What a freshly installed PC looks like: config/default.yaml, and nothing + else. + + Every `${VAR}` in that file then resolves to YAML null. The engine's own + sections already guarded against that; the camera section did not, so the + shipped config raised three pydantic errors on host, username and password + and the engine would not start at all on any machine without a developer's + .env - which is every machine the product is installed on. It was found by + cloning the repository and running this suite, never by working in a + checkout where .env is right there. + """ + _no_env(monkeypatch, tmp_path) + + root = Path(__file__).resolve().parent.parent + cfg = load_config(root / "config" / "default.yaml") + + assert cfg.api.username == "" + assert cfg.api.port == 8010 + for cam in cfg.cameras: + assert cam.host == "" and cam.username == "" and cam.password == "" + assert cam.port == 554 + + +def test_a_camera_with_no_address_is_not_seeded(tmp_path, monkeypatch): + """The bundled config declares a camera whose address comes from the + environment. On an installed PC there is none, so seeding it would put a + camera the shop never added into a brand new install, permanently failing + to connect to "" - and their first task would be working out what it was.""" + from behavision.cameras import CameraStore + + _no_env(monkeypatch, tmp_path) + + root = Path(__file__).resolve().parent.parent + cfg = load_config(root / "config" / "default.yaml") + + store = CameraStore(tmp_path / "cameras.json") + assert store.seed(cfg.cameras) is False + assert store.list() == [] + assert not (tmp_path / "cameras.json").exists()