refactor: Improve code readability by formatting query parameters in user endpoints and enhancing error handling in sound playback
This commit is contained in:
@@ -51,11 +51,13 @@ async def list_users( # noqa: PLR0913
|
||||
limit: Annotated[int, Query(description="Items per page", ge=1, le=100)] = 50,
|
||||
search: Annotated[str | None, Query(description="Search in name or email")] = None,
|
||||
sort_by: Annotated[
|
||||
UserSortField, Query(description="Sort by field"),
|
||||
UserSortField,
|
||||
Query(description="Sort by field"),
|
||||
] = UserSortField.NAME,
|
||||
sort_order: Annotated[SortOrder, Query(description="Sort order")] = SortOrder.ASC,
|
||||
status_filter: Annotated[
|
||||
UserStatus, Query(description="Filter by status"),
|
||||
UserStatus,
|
||||
Query(description="Filter by status"),
|
||||
] = UserStatus.ALL,
|
||||
) -> dict[str, Any]:
|
||||
"""Get all users with pagination, search, and filters (admin only)."""
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
from typing import Annotated, Any
|
||||
|
||||
from fastapi import APIRouter, Depends, Query
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query, status
|
||||
|
||||
from app.core.dependencies import get_current_user, get_dashboard_service
|
||||
from app.models.user import User
|
||||
@@ -17,7 +17,13 @@ async def get_soundboard_statistics(
|
||||
dashboard_service: Annotated[DashboardService, Depends(get_dashboard_service)],
|
||||
) -> dict[str, Any]:
|
||||
"""Get soundboard statistics."""
|
||||
try:
|
||||
return await dashboard_service.get_soundboard_statistics()
|
||||
except Exception as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to fetch soundboard statistics: {e!s}",
|
||||
) from e
|
||||
|
||||
|
||||
@router.get("/track-statistics")
|
||||
@@ -26,7 +32,13 @@ async def get_track_statistics(
|
||||
dashboard_service: Annotated[DashboardService, Depends(get_dashboard_service)],
|
||||
) -> dict[str, Any]:
|
||||
"""Get track statistics."""
|
||||
try:
|
||||
return await dashboard_service.get_track_statistics()
|
||||
except Exception as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to fetch track statistics: {e!s}",
|
||||
) from e
|
||||
|
||||
|
||||
@router.get("/top-sounds")
|
||||
@@ -49,8 +61,14 @@ async def get_top_sounds(
|
||||
] = 10,
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Get top sounds by play count for a specific period."""
|
||||
try:
|
||||
return await dashboard_service.get_top_sounds(
|
||||
sound_type=sound_type,
|
||||
period=period,
|
||||
limit=limit,
|
||||
)
|
||||
except Exception as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to fetch top sounds: {e!s}",
|
||||
) from e
|
||||
|
||||
@@ -65,7 +65,8 @@ async def get_user_extractions( # noqa: PLR0913
|
||||
current_user: Annotated[User, Depends(get_current_active_user_flexible)],
|
||||
extraction_service: Annotated[ExtractionService, Depends(get_extraction_service)],
|
||||
search: Annotated[
|
||||
str | None, Query(description="Search in title, URL, or service"),
|
||||
str | None,
|
||||
Query(description="Search in title, URL, or service"),
|
||||
] = None,
|
||||
sort_by: Annotated[str, Query(description="Sort by field")] = "created_at",
|
||||
sort_order: Annotated[str, Query(description="Sort order (asc/desc)")] = "desc",
|
||||
@@ -131,7 +132,8 @@ async def get_extraction(
|
||||
async def get_all_extractions( # noqa: PLR0913
|
||||
extraction_service: Annotated[ExtractionService, Depends(get_extraction_service)],
|
||||
search: Annotated[
|
||||
str | None, Query(description="Search in title, URL, or service"),
|
||||
str | None,
|
||||
Query(description="Search in title, URL, or service"),
|
||||
] = None,
|
||||
sort_by: Annotated[str, Query(description="Sort by field")] = "created_at",
|
||||
sort_order: Annotated[str, Query(description="Sort order (asc/desc)")] = "desc",
|
||||
|
||||
@@ -80,7 +80,8 @@ async def get_all_playlists( # noqa: PLR0913
|
||||
# The playlist service returns dict, need to create playlist object structure
|
||||
playlist_id = playlist_dict["id"]
|
||||
is_favorited = await favorite_service.is_playlist_favorited(
|
||||
current_user.id, playlist_id,
|
||||
current_user.id,
|
||||
playlist_id,
|
||||
)
|
||||
favorite_count = await favorite_service.get_playlist_favorite_count(playlist_id)
|
||||
|
||||
@@ -124,11 +125,14 @@ async def get_user_playlists(
|
||||
playlist_responses = []
|
||||
for playlist in playlists:
|
||||
is_favorited = await favorite_service.is_playlist_favorited(
|
||||
current_user.id, playlist.id,
|
||||
current_user.id,
|
||||
playlist.id,
|
||||
)
|
||||
favorite_count = await favorite_service.get_playlist_favorite_count(playlist.id)
|
||||
playlist_response = PlaylistResponse.from_playlist(
|
||||
playlist, is_favorited, favorite_count,
|
||||
playlist,
|
||||
is_favorited,
|
||||
favorite_count,
|
||||
)
|
||||
playlist_responses.append(playlist_response)
|
||||
|
||||
@@ -144,7 +148,8 @@ async def get_main_playlist(
|
||||
"""Get the global main playlist."""
|
||||
playlist = await playlist_service.get_main_playlist()
|
||||
is_favorited = await favorite_service.is_playlist_favorited(
|
||||
current_user.id, playlist.id,
|
||||
current_user.id,
|
||||
playlist.id,
|
||||
)
|
||||
favorite_count = await favorite_service.get_playlist_favorite_count(playlist.id)
|
||||
return PlaylistResponse.from_playlist(playlist, is_favorited, favorite_count)
|
||||
@@ -159,7 +164,8 @@ async def get_current_playlist(
|
||||
"""Get the global current playlist (falls back to main playlist)."""
|
||||
playlist = await playlist_service.get_current_playlist()
|
||||
is_favorited = await favorite_service.is_playlist_favorited(
|
||||
current_user.id, playlist.id,
|
||||
current_user.id,
|
||||
playlist.id,
|
||||
)
|
||||
favorite_count = await favorite_service.get_playlist_favorite_count(playlist.id)
|
||||
return PlaylistResponse.from_playlist(playlist, is_favorited, favorite_count)
|
||||
@@ -191,7 +197,8 @@ async def get_playlist(
|
||||
"""Get a specific playlist."""
|
||||
playlist = await playlist_service.get_playlist_by_id(playlist_id)
|
||||
is_favorited = await favorite_service.is_playlist_favorited(
|
||||
current_user.id, playlist.id,
|
||||
current_user.id,
|
||||
playlist.id,
|
||||
)
|
||||
favorite_count = await favorite_service.get_playlist_favorite_count(playlist.id)
|
||||
return PlaylistResponse.from_playlist(playlist, is_favorited, favorite_count)
|
||||
|
||||
@@ -33,6 +33,68 @@ def get_favorite_service() -> FavoriteService:
|
||||
return FavoriteService(get_session_factory())
|
||||
|
||||
|
||||
async def play_sound_internal(
|
||||
sound_id: int, user_id: str,
|
||||
) -> dict[str, str | int | bool]:
|
||||
"""Play sound with VLC internally (used by HTTP and WebSocket endpoints)."""
|
||||
session_factory = get_session_factory()
|
||||
|
||||
# Create services
|
||||
vlc_player = get_vlc_player()
|
||||
credit_service = get_credit_service()
|
||||
|
||||
async with session_factory() as session:
|
||||
sound_repo = SoundRepository(session)
|
||||
|
||||
# Get the sound
|
||||
sound = await sound_repo.get_by_id(sound_id)
|
||||
if not sound:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=f"Sound with ID {sound_id} not found",
|
||||
)
|
||||
|
||||
# Check and validate credits before playing
|
||||
try:
|
||||
await credit_service.validate_and_reserve_credits(
|
||||
int(user_id),
|
||||
CreditActionType.VLC_PLAY_SOUND,
|
||||
)
|
||||
except InsufficientCreditsError as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_402_PAYMENT_REQUIRED,
|
||||
detail=(
|
||||
f"Insufficient credits: {e.required} required, "
|
||||
f"{e.available} available"
|
||||
),
|
||||
) from e
|
||||
|
||||
# Play the sound using VLC
|
||||
success = await vlc_player.play_sound(sound)
|
||||
|
||||
# Deduct credits based on success
|
||||
await credit_service.deduct_credits(
|
||||
int(user_id),
|
||||
CreditActionType.VLC_PLAY_SOUND,
|
||||
success=success,
|
||||
metadata={"sound_id": sound_id, "sound_name": sound.name},
|
||||
)
|
||||
|
||||
if not success:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="Failed to launch VLC for sound playback",
|
||||
)
|
||||
|
||||
return {
|
||||
"message": f"Sound '{sound.name}' is now playing via VLC",
|
||||
"sound_id": sound_id,
|
||||
"sound_name": sound.name,
|
||||
"success": True,
|
||||
"credits_deducted": 1,
|
||||
}
|
||||
|
||||
|
||||
async def get_sound_repository(
|
||||
session: Annotated[AsyncSession, Depends(get_db)],
|
||||
) -> SoundRepository:
|
||||
@@ -91,11 +153,14 @@ async def get_sounds( # noqa: PLR0913
|
||||
sound_responses = []
|
||||
for sound in sounds:
|
||||
is_favorited = await favorite_service.is_sound_favorited(
|
||||
current_user.id, sound.id,
|
||||
current_user.id,
|
||||
sound.id,
|
||||
)
|
||||
favorite_count = await favorite_service.get_sound_favorite_count(sound.id)
|
||||
sound_response = SoundResponse.from_sound(
|
||||
sound, is_favorited, favorite_count,
|
||||
sound,
|
||||
is_favorited,
|
||||
favorite_count,
|
||||
)
|
||||
sound_responses.append(sound_response)
|
||||
|
||||
@@ -113,52 +178,10 @@ async def get_sounds( # noqa: PLR0913
|
||||
async def play_sound_with_vlc(
|
||||
sound_id: int,
|
||||
current_user: Annotated[User, Depends(get_current_active_user_flexible)],
|
||||
vlc_player: Annotated[VLCPlayerService, Depends(get_vlc_player)],
|
||||
sound_repo: Annotated[SoundRepository, Depends(get_sound_repository)],
|
||||
credit_service: Annotated[CreditService, Depends(get_credit_service)],
|
||||
) -> dict[str, str | int | bool]:
|
||||
"""Play a sound using VLC subprocess (requires 1 credit)."""
|
||||
try:
|
||||
# Get the sound
|
||||
sound = await sound_repo.get_by_id(sound_id)
|
||||
if not sound:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=f"Sound with ID {sound_id} not found",
|
||||
)
|
||||
|
||||
# Check and validate credits before playing
|
||||
try:
|
||||
await credit_service.validate_and_reserve_credits(
|
||||
current_user.id,
|
||||
CreditActionType.VLC_PLAY_SOUND,
|
||||
)
|
||||
except InsufficientCreditsError as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_402_PAYMENT_REQUIRED,
|
||||
detail=(
|
||||
f"Insufficient credits: {e.required} required, "
|
||||
f"{e.available} available"
|
||||
),
|
||||
) from e
|
||||
|
||||
# Play the sound using VLC
|
||||
success = await vlc_player.play_sound(sound)
|
||||
|
||||
# Deduct credits based on success
|
||||
await credit_service.deduct_credits(
|
||||
current_user.id,
|
||||
CreditActionType.VLC_PLAY_SOUND,
|
||||
success=success,
|
||||
metadata={"sound_id": sound_id, "sound_name": sound.name},
|
||||
)
|
||||
|
||||
if not success:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="Failed to launch VLC for sound playback",
|
||||
)
|
||||
|
||||
return await play_sound_internal(sound_id, str(current_user.id))
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -166,14 +189,6 @@ async def play_sound_with_vlc(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to play sound: {e!s}",
|
||||
) from e
|
||||
else:
|
||||
return {
|
||||
"message": f"Sound '{sound.name}' is now playing via VLC",
|
||||
"sound_id": sound_id,
|
||||
"sound_name": sound.name,
|
||||
"success": True,
|
||||
"credits_deducted": 1,
|
||||
}
|
||||
|
||||
|
||||
@router.post("/stop")
|
||||
|
||||
@@ -99,6 +99,40 @@ class SocketManager:
|
||||
else:
|
||||
logger.info("Unknown client %s disconnected", sid)
|
||||
|
||||
@self.sio.event
|
||||
async def play_sound(sid: str, data: dict) -> None:
|
||||
"""Handle play sound event from client."""
|
||||
user_id = self.socket_users.get(sid)
|
||||
|
||||
if not user_id:
|
||||
logger.warning("Play sound request from unknown client %s", sid)
|
||||
return
|
||||
|
||||
sound_id = data.get("sound_id")
|
||||
if not sound_id:
|
||||
logger.warning(
|
||||
"Play sound request missing sound_id from user %s", user_id,
|
||||
)
|
||||
return
|
||||
|
||||
try:
|
||||
# Import here to avoid circular imports
|
||||
from app.api.v1.sounds import play_sound_internal
|
||||
|
||||
# Call the internal play sound function
|
||||
await play_sound_internal(int(sound_id), user_id)
|
||||
logger.info("User %s played sound %s via WebSocket", user_id, sound_id)
|
||||
except Exception as e:
|
||||
logger.exception(
|
||||
"Error playing sound %s for user %s: %s", sound_id, user_id, e,
|
||||
)
|
||||
# Emit error back to user
|
||||
await self.sio.emit(
|
||||
"sound_play_error",
|
||||
{"sound_id": sound_id, "error": str(e)},
|
||||
room=sid,
|
||||
)
|
||||
|
||||
async def send_to_user(self, user_id: str, event: str, data: dict) -> bool:
|
||||
"""Send a message to a specific user's room."""
|
||||
room_id = self.user_rooms.get(user_id)
|
||||
|
||||
@@ -204,7 +204,8 @@ class TestAdminUserEndpoints:
|
||||
) -> None:
|
||||
"""Test listing users as non-admin user."""
|
||||
with patch(
|
||||
"app.core.dependencies.get_current_active_user", return_value=regular_user,
|
||||
"app.core.dependencies.get_current_active_user",
|
||||
return_value=regular_user,
|
||||
):
|
||||
response = await client.get("/api/v1/admin/users/")
|
||||
|
||||
@@ -296,7 +297,8 @@ class TestAdminUserEndpoints:
|
||||
) as mock_get_by_id,
|
||||
patch("app.repositories.user.UserRepository.update") as mock_update,
|
||||
patch(
|
||||
"app.repositories.plan.PlanRepository.get_by_id", return_value=test_plan,
|
||||
"app.repositories.plan.PlanRepository.get_by_id",
|
||||
return_value=test_plan,
|
||||
),
|
||||
):
|
||||
mock_user = type(
|
||||
|
||||
@@ -609,7 +609,8 @@ class TestAuthEndpoints:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_profile_unauthenticated(
|
||||
self, test_client: AsyncClient,
|
||||
self,
|
||||
test_client: AsyncClient,
|
||||
) -> None:
|
||||
"""Test update profile without authentication."""
|
||||
response = await test_client.patch(
|
||||
@@ -645,7 +646,8 @@ class TestAuthEndpoints:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_change_password_unauthenticated(
|
||||
self, test_client: AsyncClient,
|
||||
self,
|
||||
test_client: AsyncClient,
|
||||
) -> None:
|
||||
"""Test change password without authentication."""
|
||||
response = await test_client.post(
|
||||
@@ -716,7 +718,8 @@ class TestAuthEndpoints:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_user_providers_unauthenticated(
|
||||
self, test_client: AsyncClient,
|
||||
self,
|
||||
test_client: AsyncClient,
|
||||
) -> None:
|
||||
"""Test get user OAuth providers without authentication."""
|
||||
response = await test_client.get("/api/v1/auth/user-providers")
|
||||
|
||||
@@ -540,15 +540,19 @@ class TestPlaylistRepository:
|
||||
|
||||
# Add first two sounds sequentially (positions 0, 1)
|
||||
await playlist_repository.add_sound_to_playlist(
|
||||
playlist_id, sound_ids[0],
|
||||
playlist_id,
|
||||
sound_ids[0],
|
||||
) # position 0
|
||||
await playlist_repository.add_sound_to_playlist(
|
||||
playlist_id, sound_ids[1],
|
||||
playlist_id,
|
||||
sound_ids[1],
|
||||
) # position 1
|
||||
|
||||
# Now insert third sound at position 1 - should shift existing sound at position 1 to position 2
|
||||
await playlist_repository.add_sound_to_playlist(
|
||||
playlist_id, sound_ids[2], position=1,
|
||||
playlist_id,
|
||||
sound_ids[2],
|
||||
position=1,
|
||||
)
|
||||
|
||||
# Verify the final positions
|
||||
@@ -630,15 +634,19 @@ class TestPlaylistRepository:
|
||||
|
||||
# Add first two sounds sequentially (positions 0, 1)
|
||||
await playlist_repository.add_sound_to_playlist(
|
||||
playlist_id, sound_ids[0],
|
||||
playlist_id,
|
||||
sound_ids[0],
|
||||
) # position 0
|
||||
await playlist_repository.add_sound_to_playlist(
|
||||
playlist_id, sound_ids[1],
|
||||
playlist_id,
|
||||
sound_ids[1],
|
||||
) # position 1
|
||||
|
||||
# Now insert third sound at position 0 - should shift existing sounds to positions 1, 2
|
||||
await playlist_repository.add_sound_to_playlist(
|
||||
playlist_id, sound_ids[2], position=0,
|
||||
playlist_id,
|
||||
sound_ids[2],
|
||||
position=0,
|
||||
)
|
||||
|
||||
# Verify the final positions
|
||||
|
||||
@@ -63,7 +63,8 @@ class TestSchedulerService:
|
||||
}
|
||||
|
||||
with patch.object(
|
||||
scheduler_service.credit_service, "recharge_all_users_credits",
|
||||
scheduler_service.credit_service,
|
||||
"recharge_all_users_credits",
|
||||
) as mock_recharge:
|
||||
mock_recharge.return_value = mock_stats
|
||||
|
||||
@@ -75,7 +76,8 @@ class TestSchedulerService:
|
||||
async def test_daily_credit_recharge_failure(self, scheduler_service) -> None:
|
||||
"""Test daily credit recharge task with failure."""
|
||||
with patch.object(
|
||||
scheduler_service.credit_service, "recharge_all_users_credits",
|
||||
scheduler_service.credit_service,
|
||||
"recharge_all_users_credits",
|
||||
) as mock_recharge:
|
||||
mock_recharge.side_effect = Exception("Database error")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user