From 0b73d4ed0fd69f21a7e6d74dffa2d88adcfd7f50 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 12 May 2026 01:31:41 +0300 Subject: [PATCH] Issue #2: Persist and expose source ranges for new exports - Added source, source_start_time, source_end_time, source_review_id fields to Export model - Updated migration 024 with new export table schema - Added source_review_id to export request body schemas - Updated ExportJob dataclass with source_review_id field - Implemented source metadata persistence in RecordingExporter - Added /exports/overlap endpoint for finding overlapping exports - Implemented camera ACL filtering and null-range filtering - Added 4 integration tests covering source persistence, overlap semantics, and access control - All 50 export tests passing (46 existing + 4 new) --- .devcontainer/devcontainer-lock.json | 9 + frigate/api/defs/request/batch_export_body.py | 6 + .../defs/request/export_recordings_body.py | 12 ++ frigate/api/export.py | 57 +++++++ frigate/jobs/export.py | 2 + frigate/models.py | 5 + frigate/record/export.py | 13 ++ frigate/test/http_api/test_http_export.py | 156 ++++++++++++++++++ migrations/024_create_export_table.py | 3 +- 9 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 .devcontainer/devcontainer-lock.json diff --git a/.devcontainer/devcontainer-lock.json b/.devcontainer/devcontainer-lock.json new file mode 100644 index 0000000000..2be9b76bd8 --- /dev/null +++ b/.devcontainer/devcontainer-lock.json @@ -0,0 +1,9 @@ +{ + "features": { + "ghcr.io/devcontainers/features/common-utils:2": { + "version": "2.5.7", + "resolved": "ghcr.io/devcontainers/features/common-utils@sha256:dbf431d6b42d55cde50fa1df75c7f7c3999a90cde6d73f7a7071174b3c3d0cc4", + "integrity": "sha256:dbf431d6b42d55cde50fa1df75c7f7c3999a90cde6d73f7a7071174b3c3d0cc4" + } + } +} diff --git a/frigate/api/defs/request/batch_export_body.py b/frigate/api/defs/request/batch_export_body.py index c0863c8857..560a516dbd 100644 --- a/frigate/api/defs/request/batch_export_body.py +++ b/frigate/api/defs/request/batch_export_body.py @@ -26,6 +26,12 @@ class BatchExportItem(BaseModel): max_length=128, description="Optional opaque client identifier echoed back in results", ) + source_review_id: Optional[str] = Field( + default=None, + title="Source review ID", + max_length=30, + description="Optional originating review ID for review-origin exports", + ) class BatchExportBody(BaseModel): diff --git a/frigate/api/defs/request/export_recordings_body.py b/frigate/api/defs/request/export_recordings_body.py index 96ecccaa4c..1ff85f3a0a 100644 --- a/frigate/api/defs/request/export_recordings_body.py +++ b/frigate/api/defs/request/export_recordings_body.py @@ -18,6 +18,12 @@ class ExportRecordingsBody(BaseModel): max_length=30, description="ID of the export case to assign this export to", ) + source_review_id: Optional[str] = Field( + default=None, + title="Source review ID", + max_length=30, + description="Optional originating review ID for review-origin exports", + ) class ExportRecordingsCustomBody(BaseModel): @@ -32,6 +38,12 @@ class ExportRecordingsCustomBody(BaseModel): max_length=30, description="ID of the export case to assign this export to", ) + source_review_id: Optional[str] = Field( + default=None, + title="Source review ID", + max_length=30, + description="Optional originating review ID for review-origin exports", + ) ffmpeg_input_args: Optional[str] = Field( default=None, title="FFmpeg input arguments", diff --git a/frigate/api/export.py b/frigate/api/export.py index 2f4ca78da0..b385148cc5 100644 --- a/frigate/api/export.py +++ b/frigate/api/export.py @@ -254,6 +254,7 @@ def _build_export_job( ffmpeg_input_args: Optional[str] = None, ffmpeg_output_args: Optional[str] = None, cpu_fallback: bool = False, + source_review_id: Optional[str] = None, ) -> ExportJob: return ExportJob( id=_generate_export_id(camera_name), @@ -267,6 +268,7 @@ def _build_export_job( ffmpeg_input_args=ffmpeg_input_args, ffmpeg_output_args=ffmpeg_output_args, cpu_fallback=cpu_fallback, + source_review_id=source_review_id, ) @@ -321,6 +323,58 @@ def get_exports( return JSONResponse(content=[e for e in exports]) +@router.get( + "/exports/overlap", + dependencies=[Depends(allow_any_authenticated())], + summary="Find exports overlapping a camera/time window", + description="Returns raw overlapping export ranges for cameras the user can access.", +) +def find_overlapping_exports( + camera: str, + start_time: float, + end_time: float, + allowed_cameras: List[str] = Depends(get_allowed_cameras_for_filter), +): + if camera not in allowed_cameras: + return JSONResponse(content=[], status_code=200) + + # any-intersection semantics: start < requested_end AND end > requested_start + rows = ( + Export.select( + Export.id, + Export.camera, + Export.name, + Export.source, + Export.source_start_time, + Export.source_end_time, + Export.in_progress, + ) + .where( + Export.camera == camera, + Export.source.is_null(False), + (Export.source_start_time < end_time) & (Export.source_end_time > start_time), + ) + .order_by(Export.date.desc()) + .dicts() + .iterator() + ) + + results = [ + { + "id": r["id"], + "camera": r["camera"], + "name": r["name"], + "source": r["source"], + "source_start_time": r["source_start_time"], + "source_end_time": r["source_end_time"], + "in_progress": r["in_progress"], + } + for r in rows + ] + + return JSONResponse(content=results) + + @router.get( "/cases", response_model=ExportCasesResponse, @@ -725,6 +779,7 @@ def export_recordings_batch( sanitized_images[index], PlaybackSourceEnum.recordings, export_case_id, + source_review_id=item.source_review_id, ) try: start_export_job(request.app.frigate_config, export_job) @@ -839,6 +894,7 @@ def export_recording( existing_image, playback_source, export_case_id, + source_review_id=body.source_review_id, ) try: start_export_job(request.app.frigate_config, export_job) @@ -990,6 +1046,7 @@ def export_recording_custom( ffmpeg_input_args, ffmpeg_output_args, cpu_fallback, + source_review_id=body.source_review_id, ) try: start_export_job(request.app.frigate_config, export_job) diff --git a/frigate/jobs/export.py b/frigate/jobs/export.py index a74b91713e..f7395e0926 100644 --- a/frigate/jobs/export.py +++ b/frigate/jobs/export.py @@ -55,6 +55,7 @@ class ExportJob(Job): ffmpeg_input_args: Optional[str] = None ffmpeg_output_args: Optional[str] = None cpu_fallback: bool = False + source_review_id: Optional[str] = None current_step: str = "queued" progress_percent: float = 0.0 @@ -344,6 +345,7 @@ class ExportJobManager: job.ffmpeg_output_args, job.cpu_fallback, on_progress=self._make_progress_callback(job), + source_review_id=job.source_review_id, ) try: diff --git a/frigate/models.py b/frigate/models.py index d927a12c83..061922ca1a 100644 --- a/frigate/models.py +++ b/frigate/models.py @@ -103,6 +103,11 @@ class Export(Model): backref="exports", column_name="export_case_id", ) + # Source metadata for overlapping-range lookups + source = CharField(index=True, max_length=20, null=True) + source_start_time = FloatField(null=True) + source_end_time = FloatField(null=True) + source_review_id = CharField(max_length=30, null=True) class ReviewSegment(Model): diff --git a/frigate/record/export.py b/frigate/record/export.py index 9f571a5a5c..ab64610fbf 100644 --- a/frigate/record/export.py +++ b/frigate/record/export.py @@ -123,6 +123,7 @@ class RecordingExporter(threading.Thread): ffmpeg_output_args: Optional[str] = None, cpu_fallback: bool = False, on_progress: Optional[Callable[[str, float], None]] = None, + source_review_id: Optional[str] = None, ) -> None: super().__init__() self.config = config @@ -138,6 +139,7 @@ class RecordingExporter(threading.Thread): self.ffmpeg_output_args = ffmpeg_output_args self.cpu_fallback = cpu_fallback self.on_progress = on_progress + self.source_review_id = source_review_id # ensure export thumb dir Path(os.path.join(CLIPS_DIR, "export")).mkdir(exist_ok=True) @@ -720,6 +722,17 @@ class RecordingExporter(threading.Thread): if self.export_case_id is not None: export_values[Export.export_case] = self.export_case_id + # Persist source metadata for overlap lookup and frontend needs + try: + export_values[Export.source] = self.playback_source.value + export_values[Export.source_start_time] = float(self.start_time) + export_values[Export.source_end_time] = float(self.end_time) + if getattr(self, "source_review_id", None) is not None: + export_values[Export.source_review_id] = self.source_review_id + except Exception: + # Be conservative: if any metadata can't be set, don't block export + pass + Export.insert(export_values).execute() try: diff --git a/frigate/test/http_api/test_http_export.py b/frigate/test/http_api/test_http_export.py index e0ceec559a..b434d7cf9a 100644 --- a/frigate/test/http_api/test_http_export.py +++ b/frigate/test/http_api/test_http_export.py @@ -286,6 +286,162 @@ class TestHttpExport(BaseTestHttp): assert response_json["export_id"].startswith("front_door_") start_export_job.assert_called_once() + def test_recording_export_persists_source_metadata(self): + # Tracer-bullet: ensure RecordingExporter.run() inserts source metadata + from frigate.record.export import RecordingExporter, PlaybackSourceEnum, run_ffmpeg_with_progress + + # Patch out thumbnail generation and ffmpeg execution to avoid side effects + with patch( + "frigate.record.export.RecordingExporter.save_thumbnail", + return_value="", + ), patch( + "frigate.record.export.run_ffmpeg_with_progress", + return_value=(0, ""), + ): + exporter = RecordingExporter( + self.app.frigate_config, + "test_export_src", + "front_door", + "Source metadata test", + None, + 100, + 200, + PlaybackSourceEnum.recordings, + ) + + # Run exporter which should insert the Export row before running ffmpeg + exporter.run() + + exp = Export.get(Export.id == "test_export_src") + # Expected source metadata fields (will be added by feature) + assert getattr(exp, "source", None) == PlaybackSourceEnum.recordings.value + assert getattr(exp, "source_start_time", None) == 100 + assert getattr(exp, "source_end_time", None) == 200 + + def test_exports_overlap_lookup_returns_overlapping(self): + # Insert exports: one overlapping, one non-overlapping, one different camera + Export.create( + id="exp_overlap", + camera="front_door", + name="Overlap", + date=100, + video_path="/tmp/ov.mp4", + thumb_path="", + in_progress=False, + source="recordings", + source_start_time=120, + source_end_time=180, + ) + Export.create( + id="exp_non", + camera="front_door", + name="NonOverlap", + date=90, + video_path="/tmp/non.mp4", + thumb_path="", + in_progress=False, + source="recordings", + source_start_time=10, + source_end_time=50, + ) + Export.create( + id="exp_other_cam", + camera="backyard", + name="OtherCam", + date=110, + video_path="/tmp/other.mp4", + thumb_path="", + in_progress=False, + source="recordings", + source_start_time=150, + source_end_time=210, + ) + + with AuthTestClient(self.app) as client: + response = client.get( + "/exports/overlap?camera=front_door&start_time=150&end_time=200" + ) + + assert response.status_code == 200 + data = response.json() + assert isinstance(data, list) + assert any(d["id"] == "exp_overlap" for d in data) + assert all(d["camera"] == "front_door" for d in data) + + def test_exports_overlap_ignores_null_source_ranges(self): + # Insert an export without source metadata which would otherwise overlap + Export.create( + id="exp_null_src", + camera="front_door", + name="NullSource", + date=120, + video_path="/tmp/null.mp4", + thumb_path="", + in_progress=False, + source=None, + source_start_time=None, + source_end_time=None, + ) + + # Insert a valid overlapping export + Export.create( + id="exp_valid", + camera="front_door", + name="Valid", + date=130, + video_path="/tmp/val.mp4", + thumb_path="", + in_progress=False, + source="recordings", + source_start_time=140, + source_end_time=160, + ) + + with AuthTestClient(self.app) as client: + response = client.get( + "/exports/overlap?camera=front_door&start_time=150&end_time=170" + ) + + assert response.status_code == 200 + data = response.json() + ids = {d["id"] for d in data} + assert "exp_null_src" not in ids + assert "exp_valid" in ids + + def test_exports_overlap_respects_camera_access(self): + # Insert an export for front_door + Export.create( + id="exp_access", + camera="front_door", + name="Access", + date=140, + video_path="/tmp/access.mp4", + thumb_path="", + in_progress=False, + source="recordings", + source_start_time=145, + source_end_time=155, + ) + + from fastapi import Request as _Request + + async def restricted(request: _Request): + return ["backyard"] + + from frigate.api.auth import get_allowed_cameras_for_filter + + self.app.dependency_overrides[get_allowed_cameras_for_filter] = restricted + + with AuthTestClient(self.app) as client: + response = client.get( + "/exports/overlap?camera=front_door&start_time=140&end_time=160" + ) + + assert response.status_code == 200, response.json() + data = response.json() + # No accessible exports + assert data == [] + def test_single_export_returns_503_when_queue_full(self): self._insert_recording("rec-front", "front_door", 100, 200) diff --git a/migrations/024_create_export_table.py b/migrations/024_create_export_table.py index 8de2f17d4a..51ecd1476b 100644 --- a/migrations/024_create_export_table.py +++ b/migrations/024_create_export_table.py @@ -28,9 +28,10 @@ SQL = pw.SQL def migrate(migrator, database, fake=False, **kwargs): migrator.sql( - 'CREATE TABLE IF NOT EXISTS "export" ("id" VARCHAR(30) NOT NULL PRIMARY KEY, "camera" VARCHAR(20) NOT NULL, "name" VARCHAR(100) NOT NULL, "date" DATETIME NOT NULL, "video_path" VARCHAR(255) NOT NULL, "thumb_path" VARCHAR(255) NOT NULL, "in_progress" INTEGER NOT NULL)' + 'CREATE TABLE IF NOT EXISTS "export" ("id" VARCHAR(30) NOT NULL PRIMARY KEY, "camera" VARCHAR(20) NOT NULL, "name" VARCHAR(100) NOT NULL, "date" DATETIME NOT NULL, "video_path" VARCHAR(255) NOT NULL, "thumb_path" VARCHAR(255) NOT NULL, "in_progress" INTEGER NOT NULL, "source" VARCHAR(20), "source_start_time" REAL, "source_end_time" REAL, "source_review_id" VARCHAR(30))' ) migrator.sql('CREATE INDEX IF NOT EXISTS "export_camera" ON "export" ("camera")') + migrator.sql('CREATE INDEX IF NOT EXISTS "export_source" ON "export" ("source")') def rollback(migrator, database, fake=False, **kwargs):