Add tests for extraction API endpoints and enhance existing tests

- Implement tests for admin extraction API endpoints including status retrieval, deletion of extractions, and permission checks.
- Add tests for user extraction deletion, ensuring proper handling of permissions and non-existent extractions.
- Enhance sound endpoint tests to include duplicate handling in responses.
- Refactor favorite service tests to utilize mock dependencies for better maintainability and clarity.
- Update sound scanner tests to improve file handling and ensure proper deletion of associated files.

app/services/extraction.py

@@ -2,14 +2,16 @@
 
 import asyncio
 import shutil
+from dataclasses import dataclass
 from pathlib import Path
-from typing import TypedDict
+from typing import Any, TypedDict
 
 import yt_dlp
 from sqlmodel.ext.asyncio.session import AsyncSession
 
 from app.core.config import settings
 from app.core.logging import get_logger
+from app.models.extraction import Extraction
 from app.models.sound import Sound
 from app.repositories.extraction import ExtractionRepository
 from app.repositories.sound import SoundRepository
@@ -21,6 +23,18 @@ from app.utils.audio import get_audio_duration, get_file_hash, get_file_size
 logger = get_logger(__name__)
 
 
+@dataclass
+class ExtractionContext:
+    """Context data for extraction processing."""
+
+    extraction_id: int
+    extraction_url: str
+    extraction_service: str | None
+    extraction_service_id: str | None
+    extraction_title: str | None
+    user_id: int
+
+
 class ExtractionInfo(TypedDict):
     """Type definition for extraction information."""
 
@@ -150,8 +164,8 @@ class ExtractionService:
             logger.exception("Failed to detect service info for URL: %s", url)
             return None
 
-    async def process_extraction(self, extraction_id: int) -> ExtractionInfo:
-        """Process an extraction job."""
+    async def _validate_extraction(self, extraction_id: int) -> tuple:
+        """Validate extraction and return extraction data."""
         extraction = await self.extraction_repo.get_by_id(extraction_id)
         if not extraction:
             msg = f"Extraction {extraction_id} not found"
@@ -173,9 +187,183 @@ class ExtractionService:
             user = await self.user_repo.get_by_id(user_id)
             user_name = user.name if user else None
         except Exception:
-            logger.warning("Failed to get user %d for extraction", user_id)
+            logger.exception("Failed to get user %d for extraction", user_id)
             user_name = None
 
+        return (
+            extraction,
+            user_id,
+            extraction_url,
+            extraction_service,
+            extraction_service_id,
+            extraction_title,
+            user_name,
+        )
+
+    async def _handle_service_detection(
+        self,
+        extraction: Extraction,
+        context: ExtractionContext,
+    ) -> tuple:
+        """Handle service detection and duplicate checking."""
+        if context.extraction_service and context.extraction_service_id:
+            return (
+                context.extraction_service,
+                context.extraction_service_id,
+                context.extraction_title,
+            )
+
+        logger.info("Detecting service info for extraction %d", context.extraction_id)
+        service_info = await self._detect_service_info(context.extraction_url)
+
+        if not service_info:
+            msg = "Unable to detect service information from URL"
+            raise ValueError(msg)
+
+        # Check if extraction already exists for this service
+        service_name = service_info["service"]
+        service_id_val = service_info["service_id"]
+
+        if not service_name or not service_id_val:
+            msg = "Service info is incomplete"
+            raise ValueError(msg)
+
+        existing = await self.extraction_repo.get_by_service_and_id(
+            service_name,
+            service_id_val,
+        )
+        if existing and existing.id != context.extraction_id:
+            error_msg = (
+                f"Extraction already exists for "
+                f"{service_info['service']}:{service_info['service_id']}"
+            )
+            logger.warning(error_msg)
+            raise ValueError(error_msg)
+
+        # Update extraction with service info
+        update_data = {
+            "service": service_info["service"],
+            "service_id": service_info["service_id"],
+            "title": service_info.get("title") or context.extraction_title,
+        }
+        await self.extraction_repo.update(extraction, update_data)
+
+        # Update values for processing
+        new_service = service_info["service"]
+        new_service_id = service_info["service_id"]
+        new_title = service_info.get("title") or context.extraction_title
+
+        await self._emit_extraction_event(
+            context.user_id,
+            {
+                "extraction_id": context.extraction_id,
+                "status": "processing",
+                "title": new_title,
+                "url": context.extraction_url,
+            },
+        )
+
+        return new_service, new_service_id, new_title
+
+    async def _process_media_files(
+        self,
+        extraction_id: int,
+        extraction_url: str,
+        extraction_title: str | None,
+        extraction_service: str,
+        extraction_service_id: str,
+    ) -> int:
+        """Process media files and create sound record."""
+        # Extract audio and thumbnail
+        audio_file, thumbnail_file = await self._extract_media(
+            extraction_id,
+            extraction_url,
+        )
+
+        # Move files to final locations
+        final_audio_path, final_thumbnail_path = (
+            await self._move_files_to_final_location(
+                audio_file,
+                thumbnail_file,
+                extraction_title,
+                extraction_service,
+                extraction_service_id,
+            )
+        )
+
+        # Create Sound record
+        sound = await self._create_sound_record(
+            final_audio_path,
+            final_thumbnail_path,
+            extraction_title,
+            extraction_service,
+            extraction_service_id,
+        )
+
+        if not sound.id:
+            msg = "Sound creation failed - no ID returned"
+            raise RuntimeError(msg)
+
+        return sound.id
+
+    async def _complete_extraction(
+        self,
+        extraction: Extraction,
+        context: ExtractionContext,
+        sound_id: int,
+    ) -> None:
+        """Complete extraction processing."""
+        # Normalize the sound
+        await self._normalize_sound(sound_id)
+
+        # Add to main playlist
+        await self._add_to_main_playlist(sound_id, context.user_id)
+
+        # Update extraction with success
+        await self.extraction_repo.update(
+            extraction,
+            {
+                "status": "completed",
+                "sound_id": sound_id,
+                "error": None,
+            },
+        )
+
+        # Emit WebSocket event for completion
+        await self._emit_extraction_event(
+            context.user_id,
+            {
+                "extraction_id": context.extraction_id,
+                "status": "completed",
+                "title": context.extraction_title,
+                "url": context.extraction_url,
+                "sound_id": sound_id,
+            },
+        )
+
+    async def process_extraction(self, extraction_id: int) -> ExtractionInfo:
+        """Process an extraction job."""
+        # Validate extraction and get context data
+        (
+            extraction,
+            user_id,
+            extraction_url,
+            extraction_service,
+            extraction_service_id,
+            extraction_title,
+            user_name,
+        ) = await self._validate_extraction(extraction_id)
+
+        # Create context object for helper methods
+        context = ExtractionContext(
+            extraction_id=extraction_id,
+            extraction_url=extraction_url,
+            extraction_service=extraction_service,
+            extraction_service_id=extraction_service_id,
+            extraction_title=extraction_title,
+            user_id=user_id,
+        )
+
         logger.info("Processing extraction %d: %s", extraction_id, extraction_url)
 
         try:
@@ -184,142 +372,53 @@ class ExtractionService:
 
             # Emit WebSocket event for processing start
             await self._emit_extraction_event(
-                user_id,
+                context.user_id,
                 {
-                    "extraction_id": extraction_id,
+                    "extraction_id": context.extraction_id,
                     "status": "processing",
-                    "title": extraction_title or "Processing extraction...",
-                    "url": extraction_url,
+                    "title": context.extraction_title or "Processing extraction...",
+                    "url": context.extraction_url,
                 },
             )
 
-            # Detect service info if not already available
-            if not extraction_service or not extraction_service_id:
-                logger.info("Detecting service info for extraction %d", extraction_id)
-                service_info = await self._detect_service_info(extraction_url)
-
-                if not service_info:
-                    msg = "Unable to detect service information from URL"
-                    raise ValueError(msg)
-
-                # Check if extraction already exists for this service
-                service_name = service_info["service"]
-                service_id_val = service_info["service_id"]
-
-                if not service_name or not service_id_val:
-                    msg = "Service info is incomplete"
-                    raise ValueError(msg)
-
-                existing = await self.extraction_repo.get_by_service_and_id(
-                    service_name,
-                    service_id_val,
-                )
-                if existing and existing.id != extraction_id:
-                    error_msg = (
-                        f"Extraction already exists for "
-                        f"{service_info['service']}:{service_info['service_id']}"
-                    )
-                    logger.warning(error_msg)
-                    raise ValueError(error_msg)
-
-                # Update extraction with service info
-                update_data = {
-                    "service": service_info["service"],
-                    "service_id": service_info["service_id"],
-                    "title": service_info.get("title") or extraction_title,
-                }
-                await self.extraction_repo.update(extraction, update_data)
-
-                # Update values for processing
-                extraction_service = service_info["service"]
-                extraction_service_id = service_info["service_id"]
-                extraction_title = service_info.get("title") or extraction_title
-
-                await self._emit_extraction_event(
-                    user_id,
-                    {
-                        "extraction_id": extraction_id,
-                        "status": "processing",
-                        "title": extraction_title,
-                        "url": extraction_url,
-                    },
-                )
-
-            # Extract audio and thumbnail
-            audio_file, thumbnail_file = await self._extract_media(
-                extraction_id,
-                extraction_url,
+            # Handle service detection and duplicate checking
+            extraction_service, extraction_service_id, extraction_title = (
+                await self._handle_service_detection(extraction, context)
             )
 
-            # Move files to final locations
-            (
-                final_audio_path,
-                final_thumbnail_path,
-            ) = await self._move_files_to_final_location(
-                audio_file,
-                thumbnail_file,
-                extraction_title,
+            # Update context with potentially new values
+            context.extraction_service = extraction_service
+            context.extraction_service_id = extraction_service_id
+            context.extraction_title = extraction_title
+
+            # Process media files and create sound record
+            sound_id = await self._process_media_files(
+                context.extraction_id,
+                context.extraction_url,
+                context.extraction_title,
                 extraction_service,
                 extraction_service_id,
             )
 
-            # Create Sound record
-            sound = await self._create_sound_record(
-                final_audio_path,
-                final_thumbnail_path,
-                extraction_title,
-                extraction_service,
-                extraction_service_id,
-            )
+            # Complete extraction processing
+            await self._complete_extraction(extraction, context, sound_id)
 
-            # Store sound_id early to avoid session detachment issues
-            sound_id = sound.id
-            if not sound_id:
-                msg = "Sound creation failed - no ID returned"
-                raise RuntimeError(msg)
-
-            # Normalize the sound
-            await self._normalize_sound(sound_id)
-
-            # Add to main playlist
-            await self._add_to_main_playlist(sound_id, user_id)
-
-            # Update extraction with success
-            await self.extraction_repo.update(
-                extraction,
-                {
-                    "status": "completed",
-                    "sound_id": sound_id,
-                    "error": None,
-                },
-            )
-
-            # Emit WebSocket event for completion
-            await self._emit_extraction_event(
-                user_id,
-                {
-                    "extraction_id": extraction_id,
-                    "status": "completed",
-                    "title": extraction_title,
-                    "url": extraction_url,
-                    "sound_id": sound_id,
-                },
-            )
-
-            logger.info("Successfully processed extraction %d", extraction_id)
+            logger.info("Successfully processed extraction %d", context.extraction_id)
 
             # Get updated extraction to get latest timestamps
-            updated_extraction = await self.extraction_repo.get_by_id(extraction_id)
+            updated_extraction = await self.extraction_repo.get_by_id(
+                context.extraction_id,
+            )
             return {
-                "id": extraction_id,
-                "url": extraction_url,
+                "id": context.extraction_id,
+                "url": context.extraction_url,
                 "service": extraction_service,
                 "service_id": extraction_service_id,
                 "title": extraction_title,
                 "status": "completed",
                 "error": None,
                 "sound_id": sound_id,
-                "user_id": user_id,
+                "user_id": context.user_id,
                 "user_name": user_name,
                 "created_at": (
                     updated_extraction.created_at.isoformat()
@@ -337,18 +436,18 @@ class ExtractionService:
             error_msg = str(e)
             logger.exception(
                 "Failed to process extraction %d: %s",
-                extraction_id,
+                context.extraction_id,
                 error_msg,
             )
 
             # Emit WebSocket event for failure
             await self._emit_extraction_event(
-                user_id,
+                context.user_id,
                 {
-                    "extraction_id": extraction_id,
+                    "extraction_id": context.extraction_id,
                     "status": "failed",
-                    "title": extraction_title or "Extraction failed",
-                    "url": extraction_url,
+                    "title": context.extraction_title or "Extraction failed",
+                    "url": context.extraction_url,
                     "error": error_msg,
                 },
             )
@@ -363,17 +462,19 @@ class ExtractionService:
             )
 
             # Get updated extraction to get latest timestamps
-            updated_extraction = await self.extraction_repo.get_by_id(extraction_id)
+            updated_extraction = await self.extraction_repo.get_by_id(
+                context.extraction_id,
+            )
             return {
-                "id": extraction_id,
-                "url": extraction_url,
-                "service": extraction_service,
-                "service_id": extraction_service_id,
-                "title": extraction_title,
+                "id": context.extraction_id,
+                "url": context.extraction_url,
+                "service": context.extraction_service,
+                "service_id": context.extraction_service_id,
+                "title": context.extraction_title,
                 "status": "failed",
                 "error": error_msg,
                 "sound_id": None,
-                "user_id": user_id,
+                "user_id": context.user_id,
                 "user_name": user_name,
                 "created_at": (
                     updated_extraction.created_at.isoformat()
@@ -780,3 +881,174 @@ class ExtractionService:
             }
             for extraction, user in extraction_user_tuples
         ]
+
+    async def delete_extraction(
+        self,
+        extraction_id: int,
+        user_id: int | None = None,
+    ) -> bool:
+        """Delete an extraction and its associated sound and files.
+
+        Args:
+            extraction_id: The ID of the extraction to delete
+            user_id: Optional user ID for ownership verification (None for admin)
+
+        Returns:
+            True if deletion was successful, False if extraction not found
+
+        Raises:
+            ValueError: If user doesn't own the extraction (when user_id is provided)
+
+        """
+        logger.info(
+            "Deleting extraction: %d (user: %s)",
+            extraction_id,
+            user_id or "admin",
+        )
+
+        # Get the extraction record
+        extraction = await self.extraction_repo.get_by_id(extraction_id)
+        if not extraction:
+            logger.warning("Extraction %d not found", extraction_id)
+            return False
+
+        # Check ownership if user_id is provided (non-admin request)
+        if user_id is not None and extraction.user_id != user_id:
+            msg = "You don't have permission to delete this extraction"
+            raise ValueError(msg)
+
+        # Get associated sound if it exists and capture its attributes immediately
+        sound_data = None
+        sound_object = None
+        if extraction.sound_id:
+            sound_object = await self.sound_repo.get_by_id(extraction.sound_id)
+            if sound_object:
+                # Capture attributes immediately while session is valid
+                sound_data = {
+                    "id": sound_object.id,
+                    "type": sound_object.type,
+                    "filename": sound_object.filename,
+                    "is_normalized": sound_object.is_normalized,
+                    "normalized_filename": sound_object.normalized_filename,
+                    "thumbnail": sound_object.thumbnail,
+                }
+
+        try:
+            # Delete the extraction record first
+            await self.extraction_repo.delete(extraction)
+            logger.info("Deleted extraction record: %d", extraction_id)
+
+            # Check if sound was in current playlist before deletion
+            sound_was_in_current_playlist = False
+            if sound_object and sound_data:
+                sound_was_in_current_playlist = (
+                    await self._check_sound_in_current_playlist(sound_data["id"])
+                )
+
+            # If there's an associated sound, delete it and its files
+            if sound_object and sound_data:
+                await self._delete_sound_and_files(sound_object, sound_data)
+                logger.info(
+                    "Deleted associated sound: %d (%s)",
+                    sound_data["id"],
+                    sound_data["filename"],
+                )
+
+            # Commit the transaction
+            await self.session.commit()
+
+            # Reload player playlist if deleted sound was in current playlist
+            if sound_was_in_current_playlist and sound_data:
+                await self._reload_player_playlist()
+                logger.info(
+                    "Reloaded player playlist after deleting sound %d "
+                    "from current playlist",
+                    sound_data["id"],
+                )
+
+        except Exception:
+            # Rollback on any error
+            await self.session.rollback()
+            logger.exception("Failed to delete extraction %d", extraction_id)
+            raise
+        else:
+            return True
+
+    async def _delete_sound_and_files(
+        self,
+        sound: Sound,
+        sound_data: dict[str, Any],
+    ) -> None:
+        """Delete a sound record and all its associated files."""
+        # Collect all file paths to delete using captured attributes
+        files_to_delete = []
+
+        # Original audio file
+        if sound_data["type"] == "EXT":  # Extracted sounds
+            original_path = Path("sounds/originals/extracted") / sound_data["filename"]
+            if original_path.exists():
+                files_to_delete.append(original_path)
+
+        # Normalized file
+        if sound_data["is_normalized"] and sound_data["normalized_filename"]:
+            normalized_path = (
+                Path("sounds/normalized/extracted") / sound_data["normalized_filename"]
+            )
+            if normalized_path.exists():
+                files_to_delete.append(normalized_path)
+
+        # Thumbnail file
+        if sound_data["thumbnail"]:
+            thumbnail_path = (
+                Path(settings.EXTRACTION_THUMBNAILS_DIR) / sound_data["thumbnail"]
+            )
+            if thumbnail_path.exists():
+                files_to_delete.append(thumbnail_path)
+
+        # Delete the sound from database first
+        await self.sound_repo.delete(sound)
+
+        # Delete all associated files
+        for file_path in files_to_delete:
+            try:
+                file_path.unlink()
+                logger.info("Deleted file: %s", file_path)
+            except OSError:
+                logger.exception("Failed to delete file %s", file_path)
+                # Continue with other files even if one fails
+
+    async def _check_sound_in_current_playlist(self, sound_id: int) -> bool:
+        """Check if a sound is in the current playlist."""
+        try:
+            from app.repositories.playlist import PlaylistRepository  # noqa: PLC0415
+
+            playlist_repo = PlaylistRepository(self.session)
+            current_playlist = await playlist_repo.get_current_playlist()
+
+            if not current_playlist or not current_playlist.id:
+                return False
+
+            return await playlist_repo.is_sound_in_playlist(
+                current_playlist.id, sound_id,
+            )
+        except (ImportError, AttributeError, ValueError, RuntimeError) as e:
+            logger.warning(
+                "Failed to check if sound %s is in current playlist: %s",
+                sound_id,
+                e,
+                exc_info=True,
+            )
+            return False
+
+    async def _reload_player_playlist(self) -> None:
+        """Reload the player playlist after a sound is deleted."""
+        try:
+            # Import here to avoid circular import issues
+            from app.services.player import get_player_service  # noqa: PLC0415
+
+            player = get_player_service()
+            await player.reload_playlist()
+            logger.debug("Player playlist reloaded after sound deletion")
+        except (ImportError, AttributeError, ValueError, RuntimeError) as e:
+            # Don't fail the deletion operation if player reload fails
+            logger.warning("Failed to reload player playlist: %s", e, exc_info=True)