Add comprehensive tests for playlist service and refactor socket service tests
- Introduced a new test suite for the PlaylistService covering various functionalities including creation, retrieval, updating, and deletion of playlists. - Added tests for handling sounds within playlists, ensuring correct behavior when adding/removing sounds and managing current playlists. - Refactored socket service tests for improved readability by adjusting function signatures. - Cleaned up unnecessary whitespace in sound normalizer and sound scanner tests for consistency. - Enhanced audio utility tests to ensure accurate hash and size calculations, including edge cases for nonexistent files. - Removed redundant blank lines in cookie utility tests for cleaner code.
This commit is contained in:
@@ -14,6 +14,7 @@ from app.models.extraction import Extraction
|
||||
from app.models.sound import Sound
|
||||
from app.repositories.extraction import ExtractionRepository
|
||||
from app.repositories.sound import SoundRepository
|
||||
from app.services.playlist import PlaylistService
|
||||
from app.services.sound_normalizer import SoundNormalizerService
|
||||
from app.utils.audio import get_audio_duration, get_file_hash, get_file_size
|
||||
|
||||
@@ -41,6 +42,7 @@ class ExtractionService:
|
||||
self.session = session
|
||||
self.extraction_repo = ExtractionRepository(session)
|
||||
self.sound_repo = SoundRepository(session)
|
||||
self.playlist_service = PlaylistService(session)
|
||||
|
||||
# Ensure required directories exist
|
||||
self._ensure_directories()
|
||||
@@ -447,20 +449,18 @@ class ExtractionService:
|
||||
async def _add_to_main_playlist(self, sound: Sound, user_id: int) -> None:
|
||||
"""Add the sound to the user's main playlist."""
|
||||
try:
|
||||
# This is a placeholder - implement based on your playlist logic
|
||||
# For now, we'll just log that we would add it to the main playlist
|
||||
await self.playlist_service.add_sound_to_main_playlist(sound.id, user_id)
|
||||
logger.info(
|
||||
"Would add sound %d to main playlist for user %d",
|
||||
"Added sound %d to main playlist for user %d",
|
||||
sound.id,
|
||||
user_id,
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
except Exception:
|
||||
logger.exception(
|
||||
"Error adding sound %d to main playlist for user %d: %s",
|
||||
"Error adding sound %d to main playlist for user %d",
|
||||
sound.id,
|
||||
user_id,
|
||||
e,
|
||||
)
|
||||
# Don't fail the extraction if playlist addition fails
|
||||
|
||||
|
||||
316
app/services/playlist.py
Normal file
316
app/services/playlist.py
Normal file
@@ -0,0 +1,316 @@
|
||||
"""Playlist service for business logic operations."""
|
||||
|
||||
from typing import Any
|
||||
|
||||
from fastapi import HTTPException, status
|
||||
from sqlmodel.ext.asyncio.session import AsyncSession
|
||||
|
||||
from app.core.logging import get_logger
|
||||
from app.models.playlist import Playlist
|
||||
from app.models.sound import Sound
|
||||
from app.repositories.playlist import PlaylistRepository
|
||||
from app.repositories.sound import SoundRepository
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
|
||||
class PlaylistService:
|
||||
"""Service for playlist operations."""
|
||||
|
||||
def __init__(self, session: AsyncSession) -> None:
|
||||
"""Initialize the playlist service."""
|
||||
self.session = session
|
||||
self.playlist_repo = PlaylistRepository(session)
|
||||
self.sound_repo = SoundRepository(session)
|
||||
|
||||
async def get_playlist_by_id(self, playlist_id: int) -> Playlist:
|
||||
"""Get a playlist by ID."""
|
||||
playlist = await self.playlist_repo.get_by_id(playlist_id)
|
||||
if not playlist:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail="Playlist not found",
|
||||
)
|
||||
|
||||
return playlist
|
||||
|
||||
async def get_user_playlists(self, user_id: int) -> list[Playlist]:
|
||||
"""Get all playlists for a user."""
|
||||
return await self.playlist_repo.get_by_user_id(user_id)
|
||||
|
||||
async def get_all_playlists(self) -> list[Playlist]:
|
||||
"""Get all playlists from all users."""
|
||||
return await self.playlist_repo.get_all()
|
||||
|
||||
async def get_main_playlist(self) -> Playlist:
|
||||
"""Get the global main playlist."""
|
||||
main_playlist = await self.playlist_repo.get_main_playlist()
|
||||
|
||||
if not main_playlist:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="Main playlist not found. Make sure to run database seeding."
|
||||
)
|
||||
|
||||
return main_playlist
|
||||
|
||||
async def get_current_playlist(self, user_id: int) -> Playlist:
|
||||
"""Get the user's current playlist, fallback to main playlist."""
|
||||
current_playlist = await self.playlist_repo.get_current_playlist(user_id)
|
||||
if current_playlist:
|
||||
return current_playlist
|
||||
|
||||
# Fallback to main playlist if no current playlist is set
|
||||
return await self.get_main_playlist()
|
||||
|
||||
async def create_playlist(
|
||||
self,
|
||||
user_id: int,
|
||||
name: str,
|
||||
description: str | None = None,
|
||||
genre: str | None = None,
|
||||
is_main: bool = False,
|
||||
is_current: bool = False,
|
||||
is_deletable: bool = True,
|
||||
) -> Playlist:
|
||||
"""Create a new playlist."""
|
||||
# Check if name already exists for this user
|
||||
existing_playlist = await self.playlist_repo.get_by_name(name)
|
||||
if existing_playlist and existing_playlist.user_id == user_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="A playlist with this name already exists",
|
||||
)
|
||||
|
||||
# If this is set as current, unset the previous current playlist
|
||||
if is_current:
|
||||
await self._unset_current_playlist(user_id)
|
||||
|
||||
playlist_data = {
|
||||
"user_id": user_id,
|
||||
"name": name,
|
||||
"description": description,
|
||||
"genre": genre,
|
||||
"is_main": is_main,
|
||||
"is_current": is_current,
|
||||
"is_deletable": is_deletable,
|
||||
}
|
||||
|
||||
playlist = await self.playlist_repo.create(playlist_data)
|
||||
logger.info("Created playlist '%s' for user %s", name, user_id)
|
||||
return playlist
|
||||
|
||||
async def update_playlist(
|
||||
self,
|
||||
playlist_id: int,
|
||||
user_id: int,
|
||||
name: str | None = None,
|
||||
description: str | None = None,
|
||||
genre: str | None = None,
|
||||
is_current: bool | None = None,
|
||||
) -> Playlist:
|
||||
"""Update a playlist."""
|
||||
playlist = await self.get_playlist_by_id(playlist_id)
|
||||
|
||||
update_data: dict[str, Any] = {}
|
||||
|
||||
if name is not None:
|
||||
# Check if new name conflicts with existing playlist
|
||||
existing_playlist = await self.playlist_repo.get_by_name(name)
|
||||
if (
|
||||
existing_playlist
|
||||
and existing_playlist.id != playlist_id
|
||||
and existing_playlist.user_id == user_id
|
||||
):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="A playlist with this name already exists",
|
||||
)
|
||||
update_data["name"] = name
|
||||
|
||||
if description is not None:
|
||||
update_data["description"] = description
|
||||
|
||||
if genre is not None:
|
||||
update_data["genre"] = genre
|
||||
|
||||
if is_current is not None:
|
||||
if is_current:
|
||||
await self._unset_current_playlist(user_id)
|
||||
update_data["is_current"] = is_current
|
||||
|
||||
if update_data:
|
||||
playlist = await self.playlist_repo.update(playlist, update_data)
|
||||
logger.info("Updated playlist %s for user %s", playlist_id, user_id)
|
||||
|
||||
return playlist
|
||||
|
||||
async def delete_playlist(self, playlist_id: int, user_id: int) -> None:
|
||||
"""Delete a playlist."""
|
||||
playlist = await self.get_playlist_by_id(playlist_id)
|
||||
|
||||
if not playlist.is_deletable:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="This playlist cannot be deleted",
|
||||
)
|
||||
|
||||
# Check if this is the current playlist
|
||||
was_current = playlist.is_current
|
||||
|
||||
await self.playlist_repo.delete(playlist)
|
||||
logger.info("Deleted playlist %s for user %s", playlist_id, user_id)
|
||||
|
||||
# If the deleted playlist was current, set main playlist as current
|
||||
if was_current:
|
||||
await self._set_main_as_current(user_id)
|
||||
|
||||
async def search_playlists(self, query: str, user_id: int) -> list[Playlist]:
|
||||
"""Search user's playlists by name."""
|
||||
return await self.playlist_repo.search_by_name(query, user_id)
|
||||
|
||||
async def search_all_playlists(self, query: str) -> list[Playlist]:
|
||||
"""Search all playlists by name."""
|
||||
return await self.playlist_repo.search_by_name(query)
|
||||
|
||||
async def get_playlist_sounds(self, playlist_id: int) -> list[Sound]:
|
||||
"""Get all sounds in a playlist."""
|
||||
await self.get_playlist_by_id(playlist_id) # Verify playlist exists
|
||||
return await self.playlist_repo.get_playlist_sounds(playlist_id)
|
||||
|
||||
async def add_sound_to_playlist(
|
||||
self, playlist_id: int, sound_id: int, user_id: int, position: int | None = None
|
||||
) -> None:
|
||||
"""Add a sound to a playlist."""
|
||||
# Verify playlist exists
|
||||
await self.get_playlist_by_id(playlist_id)
|
||||
|
||||
# Verify sound exists
|
||||
sound = await self.sound_repo.get_by_id(sound_id)
|
||||
if not sound:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail="Sound not found",
|
||||
)
|
||||
|
||||
# Check if sound is already in playlist
|
||||
if await self.playlist_repo.is_sound_in_playlist(playlist_id, sound_id):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="Sound is already in this playlist",
|
||||
)
|
||||
|
||||
await self.playlist_repo.add_sound_to_playlist(playlist_id, sound_id, position)
|
||||
logger.info(
|
||||
"Added sound %s to playlist %s for user %s", sound_id, playlist_id, user_id
|
||||
)
|
||||
|
||||
async def remove_sound_from_playlist(
|
||||
self, playlist_id: int, sound_id: int, user_id: int
|
||||
) -> None:
|
||||
"""Remove a sound from a playlist."""
|
||||
# Verify playlist exists
|
||||
await self.get_playlist_by_id(playlist_id)
|
||||
|
||||
# Check if sound is in playlist
|
||||
if not await self.playlist_repo.is_sound_in_playlist(playlist_id, sound_id):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail="Sound not found in this playlist",
|
||||
)
|
||||
|
||||
await self.playlist_repo.remove_sound_from_playlist(playlist_id, sound_id)
|
||||
logger.info(
|
||||
"Removed sound %s from playlist %s for user %s",
|
||||
sound_id,
|
||||
playlist_id,
|
||||
user_id,
|
||||
)
|
||||
|
||||
async def reorder_playlist_sounds(
|
||||
self, playlist_id: int, user_id: int, sound_positions: list[tuple[int, int]]
|
||||
) -> None:
|
||||
"""Reorder sounds in a playlist."""
|
||||
# Verify playlist exists
|
||||
await self.get_playlist_by_id(playlist_id)
|
||||
|
||||
# Validate all sounds are in the playlist
|
||||
for sound_id, _ in sound_positions:
|
||||
if not await self.playlist_repo.is_sound_in_playlist(playlist_id, sound_id):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=f"Sound {sound_id} is not in this playlist",
|
||||
)
|
||||
|
||||
await self.playlist_repo.reorder_playlist_sounds(playlist_id, sound_positions)
|
||||
logger.info("Reordered sounds in playlist %s for user %s", playlist_id, user_id)
|
||||
|
||||
async def set_current_playlist(self, playlist_id: int, user_id: int) -> Playlist:
|
||||
"""Set a playlist as the current playlist."""
|
||||
playlist = await self.get_playlist_by_id(playlist_id)
|
||||
|
||||
# Unset previous current playlist
|
||||
await self._unset_current_playlist(user_id)
|
||||
|
||||
# Set new current playlist
|
||||
playlist = await self.playlist_repo.update(playlist, {"is_current": True})
|
||||
logger.info("Set playlist %s as current for user %s", playlist_id, user_id)
|
||||
return playlist
|
||||
|
||||
async def unset_current_playlist(self, user_id: int) -> None:
|
||||
"""Unset the current playlist and set main playlist as current."""
|
||||
await self._unset_current_playlist(user_id)
|
||||
await self._set_main_as_current(user_id)
|
||||
logger.info(
|
||||
"Unset current playlist and set main as current for user %s", user_id
|
||||
)
|
||||
|
||||
async def get_playlist_stats(self, playlist_id: int) -> dict[str, Any]:
|
||||
"""Get statistics for a playlist."""
|
||||
await self.get_playlist_by_id(playlist_id) # Verify playlist exists
|
||||
|
||||
sound_count = await self.playlist_repo.get_playlist_sound_count(playlist_id)
|
||||
sounds = await self.playlist_repo.get_playlist_sounds(playlist_id)
|
||||
|
||||
total_duration = sum(sound.duration or 0 for sound in sounds)
|
||||
total_plays = sum(sound.play_count or 0 for sound in sounds)
|
||||
|
||||
return {
|
||||
"sound_count": sound_count,
|
||||
"total_duration_ms": total_duration,
|
||||
"total_play_count": total_plays,
|
||||
}
|
||||
|
||||
async def add_sound_to_main_playlist(self, sound_id: int, user_id: int) -> None:
|
||||
"""Add a sound to the global main playlist."""
|
||||
main_playlist = await self.get_main_playlist()
|
||||
|
||||
if main_playlist.id is None:
|
||||
raise ValueError("Main playlist has no ID")
|
||||
|
||||
# Check if sound is already in main playlist
|
||||
if not await self.playlist_repo.is_sound_in_playlist(
|
||||
main_playlist.id, sound_id
|
||||
):
|
||||
await self.playlist_repo.add_sound_to_playlist(main_playlist.id, sound_id)
|
||||
logger.info(
|
||||
"Added sound %s to main playlist for user %s",
|
||||
sound_id,
|
||||
user_id,
|
||||
)
|
||||
|
||||
async def _unset_current_playlist(self, user_id: int) -> None:
|
||||
"""Unset the current playlist for a user."""
|
||||
current_playlist = await self.playlist_repo.get_current_playlist(user_id)
|
||||
if current_playlist:
|
||||
await self.playlist_repo.update(current_playlist, {"is_current": False})
|
||||
|
||||
async def _set_main_as_current(self, user_id: int) -> None:
|
||||
"""Unset current playlist so main playlist becomes the fallback current."""
|
||||
# Just ensure no user playlist is marked as current
|
||||
# The get_current_playlist method will fallback to main playlist
|
||||
await self._unset_current_playlist(user_id)
|
||||
logger.info(
|
||||
"Unset current playlist for user %s, main playlist is now fallback",
|
||||
user_id,
|
||||
)
|
||||
@@ -107,7 +107,6 @@ class SoundNormalizerService:
|
||||
original_dir = type_to_original_dir.get(sound_type, "sounds/originals/other")
|
||||
return Path(original_dir) / filename
|
||||
|
||||
|
||||
async def _normalize_audio_one_pass(
|
||||
self,
|
||||
input_path: Path,
|
||||
@@ -178,9 +177,12 @@ class SoundNormalizerService:
|
||||
result = ffmpeg.run(stream, capture_stderr=True, quiet=True)
|
||||
analysis_output = result[1].decode("utf-8")
|
||||
except ffmpeg.Error as e:
|
||||
logger.error("FFmpeg first pass failed for %s. Stdout: %s, Stderr: %s",
|
||||
input_path, e.stdout.decode() if e.stdout else "None",
|
||||
e.stderr.decode() if e.stderr else "None")
|
||||
logger.error(
|
||||
"FFmpeg first pass failed for %s. Stdout: %s, Stderr: %s",
|
||||
input_path,
|
||||
e.stdout.decode() if e.stdout else "None",
|
||||
e.stderr.decode() if e.stderr else "None",
|
||||
)
|
||||
raise
|
||||
|
||||
# Extract loudnorm measurements from the output
|
||||
@@ -190,19 +192,28 @@ class SoundNormalizerService:
|
||||
# Find JSON in the output
|
||||
json_match = re.search(r'\{[^{}]*"input_i"[^{}]*\}', analysis_output)
|
||||
if not json_match:
|
||||
logger.error("Could not find JSON in loudnorm output: %s", analysis_output)
|
||||
logger.error(
|
||||
"Could not find JSON in loudnorm output: %s", analysis_output
|
||||
)
|
||||
raise ValueError("Could not extract loudnorm analysis data")
|
||||
|
||||
logger.debug("Found JSON match: %s", json_match.group())
|
||||
analysis_data = json.loads(json_match.group())
|
||||
|
||||
|
||||
# Check for invalid values that would cause second pass to fail
|
||||
invalid_values = ["-inf", "inf", "nan"]
|
||||
for key in ["input_i", "input_lra", "input_tp", "input_thresh", "target_offset"]:
|
||||
for key in [
|
||||
"input_i",
|
||||
"input_lra",
|
||||
"input_tp",
|
||||
"input_thresh",
|
||||
"target_offset",
|
||||
]:
|
||||
if str(analysis_data.get(key, "")).lower() in invalid_values:
|
||||
logger.warning(
|
||||
"Invalid analysis value for %s: %s. Falling back to one-pass normalization.",
|
||||
key, analysis_data.get(key)
|
||||
"Invalid analysis value for %s: %s. Falling back to one-pass normalization.",
|
||||
key,
|
||||
analysis_data.get(key),
|
||||
)
|
||||
# Fall back to one-pass normalization
|
||||
await self._normalize_audio_one_pass(input_path, output_path)
|
||||
@@ -241,9 +252,12 @@ class SoundNormalizerService:
|
||||
ffmpeg.run(stream, quiet=True, overwrite_output=True)
|
||||
logger.info("Two-pass normalization completed: %s", output_path)
|
||||
except ffmpeg.Error as e:
|
||||
logger.error("FFmpeg second pass failed for %s. Stdout: %s, Stderr: %s",
|
||||
input_path, e.stdout.decode() if e.stdout else "None",
|
||||
e.stderr.decode() if e.stderr else "None")
|
||||
logger.error(
|
||||
"FFmpeg second pass failed for %s. Stdout: %s, Stderr: %s",
|
||||
input_path,
|
||||
e.stdout.decode() if e.stdout else "None",
|
||||
e.stderr.decode() if e.stderr else "None",
|
||||
)
|
||||
raise
|
||||
|
||||
except Exception as e:
|
||||
|
||||
@@ -56,7 +56,6 @@ class SoundScannerService:
|
||||
".aac",
|
||||
}
|
||||
|
||||
|
||||
def extract_name_from_filename(self, filename: str) -> str:
|
||||
"""Extract a clean name from filename."""
|
||||
# Remove extension
|
||||
|
||||
Reference in New Issue
Block a user