diff --git a/application/backend/app/api/dependencies.py b/application/backend/app/api/dependencies.py index 911f2ea4b5..3a7cdcb568 100644 --- a/application/backend/app/api/dependencies.py +++ b/application/backend/app/api/dependencies.py @@ -150,13 +150,6 @@ def get_sink_service( return SinkService(event_bus=event_bus, db_session=db) -def get_source_update_service( - event_bus: Annotated[EventBus, Depends(get_event_bus)], db: Annotated[Session, Depends(get_db)] -) -> SourceUpdateService: - """Provides a SourceUpdateService instance.""" - return SourceUpdateService(event_bus=event_bus, db_session=db) - - def get_source_media_service( source_media_dir: Annotated[Path, Depends(get_source_media_dir)], ) -> SourceMediaService: @@ -164,6 +157,15 @@ def get_source_media_service( return SourceMediaService(source_media_dir) +def get_source_update_service( + event_bus: Annotated[EventBus, Depends(get_event_bus)], + db: Annotated[Session, Depends(get_db)], + source_media_service: Annotated[SourceMediaService, Depends(get_source_media_service)], +) -> SourceUpdateService: + """Provides a SourceUpdateService instance.""" + return SourceUpdateService(event_bus=event_bus, db_session=db, source_media_service=source_media_service) + + def get_source_status_service( scheduler: Annotated[Scheduler, Depends(get_scheduler)], ) -> SourceStatusService: diff --git a/application/backend/app/services/source_media_service.py b/application/backend/app/services/source_media_service.py index cf3583f825..abe19b0aa0 100644 --- a/application/backend/app/services/source_media_service.py +++ b/application/backend/app/services/source_media_service.py @@ -75,3 +75,34 @@ def _perform_copy() -> None: raise return target_path.resolve() + + def delete_video(self, video_path: str) -> None: + """ + Remove a previously uploaded video and its dedicated UUID subdirectory. + + This is a best-effort cleanup helper: it only removes files that live inside the + configured source media root, in their own UUID subdirectory (as created by + `upload`). Any path outside that root, or sitting directly at its root without a + subdirectory, is left untouched. + + Args: + video_path: Absolute or relative path to the video file to remove, as previously + returned by `upload` (and stored as a video_file source's 'video_path'). + + Raises: + OSError: If removing the subdirectory fails (e.g. permissions, file in use). + Callers are expected to treat this as best-effort and handle it themselves. + """ + resolved = Path(video_path).resolve() + base = self._source_media_dir.resolve() + + if not resolved.is_relative_to(base): + return + + upload_dir = resolved.parent + + # Only remove per-upload UUID subdirectories created by `upload` (base//). + if upload_dir.parent != base: + return + + shutil.rmtree(upload_dir) diff --git a/application/backend/app/services/source_service.py b/application/backend/app/services/source_service.py index 477866c93d..400366bcaf 100644 --- a/application/backend/app/services/source_service.py +++ b/application/backend/app/services/source_service.py @@ -3,12 +3,13 @@ import time from uuid import UUID +from loguru import logger from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from app.db.schema import ProjectDB, SourceDB from app.models import Source, SourceType -from app.models.source import SourceAdapter, SourceConfig, SourceTestResult +from app.models.source import SourceAdapter, SourceConfig, SourceTestResult, VideoFileConfig from app.repositories import SourceRepository from app.repositories.base import PrimaryKeyIntegrityError, UniqueConstraintIntegrityError from app.repositories.pipeline_repo import PipelineRepository @@ -22,12 +23,14 @@ ) from .event.event_bus import EventBus, EventType from .parent_process_guard import parent_process_only +from .source_media_service import SourceMediaService from .video_stream_service import VideoStreamService class SourceService: - def __init__(self, db_session: Session): + def __init__(self, db_session: Session, source_media_service: SourceMediaService | None = None): self._db_session = db_session + self._source_media_service = source_media_service @parent_process_only def create_source( @@ -89,6 +92,12 @@ def delete_source(self, source: Source) -> None: except IntegrityError: raise ResourceInUseError(ResourceType.SOURCE, str(source.id)) + match source.source_type: + case SourceType.VIDEO_FILE: + self._delete_video_best_effort(source.config_data.video_path) + case _: + pass + def get_active_source(self) -> Source | None: db_source = SourceRepository(self._db_session).get_active_source() return SourceAdapter.validate_python(db_source, from_attributes=True) if db_source else None @@ -97,11 +106,25 @@ def get_active_source_id(self) -> UUID | None: id = SourceRepository(self._db_session).get_active_source_id() return UUID(id) if id else None + def _delete_video_best_effort(self, video_path: str) -> None: + """Delete an uploaded video file, tolerating any filesystem failure.""" + if self._source_media_service is None: + return + try: + self._source_media_service.delete_video(video_path) + except OSError as e: + logger.warning("Failed to delete video file '{}': {}", video_path, e) + class SourceUpdateService(SourceService): - def __init__(self, event_bus: EventBus, db_session: Session): + def __init__( + self, + event_bus: EventBus, + db_session: Session, + source_media_service: SourceMediaService | None = None, + ): self._event_bus: EventBus = event_bus - super().__init__(db_session) + super().__init__(db_session, source_media_service) @parent_process_only def update_source( @@ -122,10 +145,18 @@ def update_source( active_source_id = self.get_active_source_id() if active_source_id == UUID(db_source.id): self._event_bus.emit_event(EventType.SOURCE_CHANGED) - return SourceAdapter.validate_python(db_source, from_attributes=True) except UniqueConstraintIntegrityError: raise ResourceWithNameAlreadyExistsError(ResourceType.SOURCE, new_name) + match source.source_type: + case SourceType.VIDEO_FILE if isinstance(new_config_data, VideoFileConfig): + if source.config_data.video_path != new_config_data.video_path: + self._delete_video_best_effort(source.config_data.video_path) + case _: + pass + + return SourceAdapter.validate_python(db_source, from_attributes=True) + _TEST_TIMEOUT_MS = 5000 def test_source(self, source: Source) -> SourceTestResult: diff --git a/application/backend/tests/integration/services/test_source_service.py b/application/backend/tests/integration/services/test_source_service.py index b60fc64dbb..88bf58017d 100644 --- a/application/backend/tests/integration/services/test_source_service.py +++ b/application/backend/tests/integration/services/test_source_service.py @@ -21,6 +21,7 @@ from app.services import ResourceInUseError, ResourceType, SourceUpdateService from app.services.base import ResourceWithIdAlreadyExistsError, ResourceWithNameAlreadyExistsError from app.services.event.event_bus import EventType +from app.services.source_media_service import SourceMediaService @pytest.fixture @@ -29,6 +30,17 @@ def fxt_source_update_service(fxt_event_bus, db_session) -> SourceUpdateService: return SourceUpdateService(fxt_event_bus, db_session) +@pytest.fixture +def fxt_source_media_service() -> MagicMock: + return MagicMock(spec=SourceMediaService) + + +@pytest.fixture +def fxt_source_update_service_with_media(fxt_event_bus, db_session, fxt_source_media_service) -> SourceUpdateService: + """Fixture to provide a SourceUpdateService instance wired with a mocked SourceMediaService.""" + return SourceUpdateService(fxt_event_bus, db_session, fxt_source_media_service) + + class TestSourceUpdateServiceIntegration: """Integration tests for ConfigurationService.""" @@ -225,6 +237,72 @@ def test_update_source_notify( assert db_source.config_data["video_path"] == "/new/path" fxt_event_bus.emit_event.assert_called_once_with(EventType.SOURCE_CHANGED) + def test_update_source_video_file_path_changed_deletes_old_video( + self, + fxt_db_sources, + fxt_source_update_service_with_media, + fxt_source_media_service, + db_session, + ): + """Changing a video_file source's video_path deletes the old (not the new) file.""" + db_source = fxt_db_sources[0] # video_file source, video_path="/path/to/video.mp4" + db_session.add(db_source) + db_session.flush() + + source = SourceAdapter.validate_python(db_source, from_attributes=True) + + fxt_source_update_service_with_media.update_source( + source=source, + new_name=source.name, + new_config_data=VideoFileConfig(video_path="/new/path"), + ) + + fxt_source_media_service.delete_video.assert_called_once_with("/path/to/video.mp4") + + def test_update_source_video_file_path_unchanged_does_not_delete_video( + self, + fxt_db_sources, + fxt_source_update_service_with_media, + fxt_source_media_service, + db_session, + ): + """Updating a video_file source without changing video_path must not delete anything.""" + db_source = fxt_db_sources[0] # video_file source, video_path="/path/to/video.mp4" + db_session.add(db_source) + db_session.flush() + + source = SourceAdapter.validate_python(db_source, from_attributes=True) + + fxt_source_update_service_with_media.update_source( + source=source, + new_name="Renamed Source", + new_config_data=VideoFileConfig(video_path="/path/to/video.mp4"), + ) + + fxt_source_media_service.delete_video.assert_not_called() + + def test_update_source_non_video_file_does_not_delete_video( + self, + fxt_db_sources, + fxt_source_update_service_with_media, + fxt_source_media_service, + db_session, + ): + """Updating a non-video_file source never touches the media service.""" + db_source = fxt_db_sources[1] # usb_camera source + db_session.add(db_source) + db_session.flush() + + source = SourceAdapter.validate_python(db_source, from_attributes=True) + + fxt_source_update_service_with_media.update_source( + source=source, + new_name="Renamed Source", + new_config_data=USBCameraConfig(device_id=2, codec=None), + ) + + fxt_source_media_service.delete_video.assert_not_called() + def test_delete_source(self, fxt_db_sources, fxt_source_update_service, db_session): """Test deleting a source.""" db_source = fxt_db_sources[0] @@ -266,6 +344,64 @@ def test_delete_source_in_use( assert "Test Detection Project" in str(exc_info.value) assert "running" in str(exc_info.value) + def test_delete_source_video_file_deletes_video( + self, + fxt_db_sources, + fxt_source_update_service_with_media, + fxt_source_media_service, + db_session, + ): + """Deleting a video_file source removes its uploaded video file too.""" + db_source = fxt_db_sources[0] # video_file source, see fxt_db_sources + db_session.add(db_source) + db_session.flush() + + source = SourceAdapter.validate_python(db_source, from_attributes=True) + + fxt_source_update_service_with_media.delete_source(source) + + assert db_session.query(SourceDB).count() == 0 + fxt_source_media_service.delete_video.assert_called_once_with("/path/to/video.mp4") + + def test_delete_source_non_video_file_does_not_delete_video( + self, + fxt_db_sources, + fxt_source_update_service_with_media, + fxt_source_media_service, + db_session, + ): + """Deleting a non-video_file source never touches the media service.""" + db_source = fxt_db_sources[1] # usb_camera source + db_session.add(db_source) + db_session.flush() + + source = SourceAdapter.validate_python(db_source, from_attributes=True) + + fxt_source_update_service_with_media.delete_source(source) + + assert db_session.query(SourceDB).count() == 0 + fxt_source_media_service.delete_video.assert_not_called() + + def test_delete_source_video_file_delete_video_failure_is_best_effort( + self, + fxt_db_sources, + fxt_source_update_service_with_media, + fxt_source_media_service, + db_session, + ): + """A filesystem failure while deleting the video must not affect the DB delete.""" + fxt_source_media_service.delete_video.side_effect = OSError("permission denied") + db_source = fxt_db_sources[0] # video_file source + db_session.add(db_source) + db_session.flush() + + source = SourceAdapter.validate_python(db_source, from_attributes=True) + + fxt_source_update_service_with_media.delete_source(source) + + assert db_session.query(SourceDB).count() == 0 + fxt_source_media_service.delete_video.assert_called_once_with("/path/to/video.mp4") + def test_test_source_video_file_exists(self, fxt_source_update_service, tmp_path): """Test test_source with a valid video file path (file exists but may not be a valid video).""" video_file = tmp_path / "test.mp4" diff --git a/application/backend/tests/unit/services/test_source_media_service.py b/application/backend/tests/unit/services/test_source_media_service.py index 9a56ec9e5f..83fa3bc07c 100644 --- a/application/backend/tests/unit/services/test_source_media_service.py +++ b/application/backend/tests/unit/services/test_source_media_service.py @@ -81,3 +81,66 @@ def _boom(*args, **kwargs): # The whole per-upload UUID subdirectory should be removed, not just the temp file. assert list(tmp_path.iterdir()) == [] + + @pytest.mark.asyncio + async def test_delete_video_removes_upload_subdirectory( + self, tmp_path: Path, fxt_source_media_service: SourceMediaService + ): + video_path = await fxt_source_media_service.upload(filename="sample.mp4", file_obj=BytesIO(b"data")) + upload_dir = video_path.parent + assert upload_dir.is_dir() + + fxt_source_media_service.delete_video(str(video_path)) + + assert not upload_dir.exists() + assert list(tmp_path.iterdir()) == [] + + def test_delete_video_noop_when_outside_source_media_dir( + self, tmp_path: Path, fxt_source_media_service: SourceMediaService + ): + outside_dir = tmp_path.parent / f"outside-{tmp_path.name}" + outside_dir.mkdir() + outside_file = outside_dir / "video.mp4" + outside_file.write_bytes(b"data") + + fxt_source_media_service.delete_video(str(outside_file)) + + assert outside_file.exists() + + def test_delete_video_noop_when_path_is_directly_under_root( + self, tmp_path: Path, fxt_source_media_service: SourceMediaService + ): + root_level_file = tmp_path / "video.mp4" + root_level_file.write_bytes(b"data") + + fxt_source_media_service.delete_video(str(root_level_file)) + + # No UUID subdirectory boundary, so nothing should be removed (avoids ever + # rmtree-ing the whole source_media_dir root). + assert root_level_file.exists() + + def test_delete_video_noop_when_path_is_source_media_root_itself(self, tmp_path: Path): + # Use a nested dir so a buggy implementation can't accidentally delete pytest's temp root. + source_media_root = tmp_path / "source_media" + source_media_root.mkdir() + + service = SourceMediaService(source_media_dir=source_media_root) + service.delete_video(str(source_media_root)) + + assert source_media_root.exists() + + @pytest.mark.asyncio + async def test_delete_video_propagates_oserror( + self, + fxt_source_media_service: SourceMediaService, + monkeypatch: pytest.MonkeyPatch, + ): + video_path = await fxt_source_media_service.upload(filename="sample.mp4", file_obj=BytesIO(b"data")) + + def _boom(*args, **kwargs): + raise OSError("permission denied") + + monkeypatch.setattr("shutil.rmtree", _boom) + + with pytest.raises(OSError, match="permission denied"): + fxt_source_media_service.delete_video(str(video_path))