diff --git a/api/src/rehearsalhub/config.py b/api/src/rehearsalhub/config.py index 67f193a..5be2b29 100644 --- a/api/src/rehearsalhub/config.py +++ b/api/src/rehearsalhub/config.py @@ -17,11 +17,6 @@ class Settings(BaseSettings): redis_url: str = "redis://localhost:6379/0" job_queue_key: str = "rh:jobs" - # Nextcloud - nextcloud_url: str = "http://nextcloud" - nextcloud_user: str = "ncadmin" - nextcloud_pass: str = "" - # App domain: str = "localhost" debug: bool = False diff --git a/api/src/rehearsalhub/routers/bands.py b/api/src/rehearsalhub/routers/bands.py index 1b8ba99..0190acd 100644 --- a/api/src/rehearsalhub/routers/bands.py +++ b/api/src/rehearsalhub/routers/bands.py @@ -9,6 +9,7 @@ from rehearsalhub.dependencies import get_current_member from rehearsalhub.schemas.band import BandCreate, BandRead, BandReadWithMembers, BandUpdate from rehearsalhub.repositories.band import BandRepository from rehearsalhub.services.band import BandService +from rehearsalhub.storage.nextcloud import NextcloudClient router = APIRouter(prefix="/bands", tags=["bands"]) @@ -29,7 +30,8 @@ async def create_band( session: AsyncSession = Depends(get_session), current_member: Member = Depends(get_current_member), ): - svc = BandService(session) + storage = NextcloudClient.for_member(current_member) + svc = BandService(session, storage) try: band = await svc.create_band(data, current_member.id, creator=current_member) except ValueError as e: @@ -45,7 +47,8 @@ async def get_band( session: AsyncSession = Depends(get_session), current_member: Member = Depends(get_current_member), ): - svc = BandService(session) + storage = NextcloudClient.for_member(current_member) + svc = BandService(session, storage) try: await svc.assert_membership(band_id, current_member.id) except PermissionError: diff --git a/api/src/rehearsalhub/routers/internal.py b/api/src/rehearsalhub/routers/internal.py index 7755c66..3b79248 100644 --- a/api/src/rehearsalhub/routers/internal.py +++ b/api/src/rehearsalhub/routers/internal.py @@ -117,7 +117,16 @@ async def nc_upload( ) uploader_id = result.scalar_one_or_none() - song_svc = SongService(session) + # Get the uploader's storage credentials + storage = None + if uploader_id: + uploader_result = await session.execute( + select(Member).where(Member.id == uploader_id).limit(1) + ) + uploader = uploader_result.scalar_one_or_none() + storage = NextcloudClient.for_member(uploader) if uploader else None + + song_svc = SongService(session, storage=storage) version = await song_svc.register_version( song.id, AudioVersionCreate( diff --git a/api/src/rehearsalhub/routers/songs.py b/api/src/rehearsalhub/routers/songs.py index dc5d2a7..ea4f40b 100644 --- a/api/src/rehearsalhub/routers/songs.py +++ b/api/src/rehearsalhub/routers/songs.py @@ -48,7 +48,8 @@ async def list_songs( await band_svc.assert_membership(band_id, current_member.id) except PermissionError: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Not a member") - song_svc = SongService(session) + storage = NextcloudClient.for_member(current_member) + song_svc = SongService(session, storage=storage) return await song_svc.list_songs(band_id) @@ -131,7 +132,8 @@ async def create_song( if band is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Band not found") - song_svc = SongService(session) + storage = NextcloudClient.for_member(current_member) + song_svc = SongService(session, storage=storage) song = await song_svc.create_song(band_id, data, current_member.id, band.slug, creator=current_member) read = SongRead.model_validate(song) read.version_count = 0 diff --git a/api/src/rehearsalhub/routers/versions.py b/api/src/rehearsalhub/routers/versions.py index bdc34a1..ea87a91 100644 --- a/api/src/rehearsalhub/routers/versions.py +++ b/api/src/rehearsalhub/routers/versions.py @@ -1,7 +1,9 @@ import uuid +import asyncio from pathlib import Path from typing import Any +import httpx from fastapi import APIRouter, Depends, HTTPException, Query, Request, status from fastapi.responses import Response from fastapi.security.utils import get_authorization_scheme_param @@ -33,6 +35,45 @@ _AUDIO_CONTENT_TYPES: dict[str, str] = { } +async def _download_with_retry(storage: NextcloudClient, file_path: str, max_retries: int = 3) -> bytes: + """Download file from Nextcloud with retry logic for transient errors.""" + last_error = None + + for attempt in range(max_retries): + try: + data = await storage.download(file_path) + return data + except httpx.ConnectError as e: + last_error = e + if attempt < max_retries - 1: + # Exponential backoff: 1s, 2s, 4s + wait_time = 2 ** attempt + await asyncio.sleep(wait_time) + continue + except httpx.HTTPStatusError as e: + # Don't retry on 4xx errors (client errors) + if e.response.status_code >= 500: + last_error = e + if attempt < max_retries - 1: + wait_time = 2 ** attempt + await asyncio.sleep(wait_time) + continue + else: + raise + except Exception as e: + last_error = e + break + + # If we exhausted retries, raise the last error + if last_error: + raise last_error + else: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Failed to download file from storage" + ) + + async def _member_from_request( request: Request, token: str | None = Query(None), @@ -124,7 +165,8 @@ async def create_version( except PermissionError: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Not a member") - song_svc = SongService(session) + storage = NextcloudClient.for_member(current_member) + song_svc = SongService(session, storage=storage) version = await song_svc.register_version(song_id, data, current_member.id) return AudioVersionRead.model_validate(version) @@ -138,8 +180,36 @@ async def get_waveform( version, _ = await _get_version_and_assert_band_membership(version_id, session, current_member) if not version.waveform_url: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Waveform not ready") - storage = NextcloudClient() - data = await storage.download(version.waveform_url) + + storage = NextcloudClient.for_member(current_member) + if storage is None: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="No storage provider configured for this account" + ) + try: + data = await _download_with_retry(storage, version.waveform_url) + except httpx.ConnectError as e: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail=f"Failed to connect to storage: {str(e)}" + ) + except httpx.HTTPStatusError as e: + if e.response.status_code == 404: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Waveform file not found in storage" + ) + else: + raise HTTPException( + status_code=status.HTTP_502_BAD_GATEWAY, + detail=f"Storage error: {str(e)}" + ) + except Exception as e: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to fetch waveform: {str(e)}" + ) import json return json.loads(data) @@ -161,7 +231,35 @@ async def stream_version( else: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="No audio file") - storage = NextcloudClient() - data = await storage.download(file_path) + storage = NextcloudClient.for_member(current_member) + if storage is None: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="No storage provider configured for this account" + ) + try: + data = await _download_with_retry(storage, file_path) + except httpx.ConnectError as e: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail=f"Failed to connect to storage: {str(e)}" + ) + except httpx.HTTPStatusError as e: + if e.response.status_code == 404: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="File not found in storage" + ) + else: + raise HTTPException( + status_code=status.HTTP_502_BAD_GATEWAY, + detail=f"Storage error: {str(e)}" + ) + except Exception as e: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to stream file: {str(e)}" + ) + content_type = _AUDIO_CONTENT_TYPES.get(Path(file_path).suffix.lower(), "application/octet-stream") return Response(content=data, media_type=content_type) diff --git a/api/src/rehearsalhub/services/band.py b/api/src/rehearsalhub/services/band.py index ad08348..c2fbb91 100644 --- a/api/src/rehearsalhub/services/band.py +++ b/api/src/rehearsalhub/services/band.py @@ -16,7 +16,7 @@ log = logging.getLogger(__name__) class BandService: def __init__(self, session: AsyncSession, storage: NextcloudClient | None = None) -> None: self._repo = BandRepository(session) - self._storage = storage or NextcloudClient() + self._storage = storage async def create_band( self, diff --git a/api/src/rehearsalhub/services/song.py b/api/src/rehearsalhub/services/song.py index 47c7ed5..db674e7 100644 --- a/api/src/rehearsalhub/services/song.py +++ b/api/src/rehearsalhub/services/song.py @@ -24,7 +24,7 @@ class SongService: self._version_repo = AudioVersionRepository(session) self._session = session self._queue = job_queue or RedisJobQueue(session) - self._storage = storage or NextcloudClient() + self._storage = storage async def create_song( self, band_id: uuid.UUID, data: SongCreate, creator_id: uuid.UUID, band_slug: str, diff --git a/api/src/rehearsalhub/storage/nextcloud.py b/api/src/rehearsalhub/storage/nextcloud.py index fcb4c9e..f54db86 100644 --- a/api/src/rehearsalhub/storage/nextcloud.py +++ b/api/src/rehearsalhub/storage/nextcloud.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging import xml.etree.ElementTree as ET from typing import Any @@ -10,31 +11,34 @@ import httpx from rehearsalhub.config import get_settings from rehearsalhub.storage.protocol import FileMetadata +logger = logging.getLogger(__name__) + _DAV_NS = "{DAV:}" class NextcloudClient: def __init__( self, - base_url: str | None = None, - username: str | None = None, - password: str | None = None, + base_url: str, + username: str, + password: str, ) -> None: - s = get_settings() - self._base = (base_url or s.nextcloud_url).rstrip("/") - self._auth = (username or s.nextcloud_user, password or s.nextcloud_pass) + if not base_url or not username: + raise ValueError("Nextcloud credentials must be provided explicitly") + self._base = base_url.rstrip("/") + self._auth = (username, password) self._dav_root = f"{self._base}/remote.php/dav/files/{self._auth[0]}" @classmethod - def for_member(cls, member: object) -> "NextcloudClient": - """Return a client using member's personal NC credentials if configured, - falling back to the global env-var credentials.""" + def for_member(cls, member: object) -> "NextcloudClient | None": + """Return a client using member's personal NC credentials if configured. + Returns None if member has no Nextcloud configuration.""" nc_url = getattr(member, "nc_url", None) nc_username = getattr(member, "nc_username", None) nc_password = getattr(member, "nc_password", None) if nc_url and nc_username and nc_password: return cls(base_url=nc_url, username=nc_username, password=nc_password) - return cls() + return None def _client(self) -> httpx.AsyncClient: return httpx.AsyncClient(auth=self._auth, timeout=30.0) @@ -83,10 +87,23 @@ class NextcloudClient: return _parse_propfind_multi(resp.text) async def download(self, path: str) -> bytes: - async with self._client() as c: - resp = await c.get(self._dav_url(path)) - resp.raise_for_status() - return resp.content + logger.debug("Downloading file from Nextcloud: %s", path) + try: + async with self._client() as c: + resp = await c.get(self._dav_url(path)) + resp.raise_for_status() + logger.debug("Successfully downloaded file: %s", path) + return resp.content + except httpx.ConnectError as e: + logger.error("Failed to connect to Nextcloud at %s: %s", self._base, str(e)) + raise + except httpx.HTTPStatusError as e: + logger.error("Nextcloud request failed for %s: %s (status: %d)", + path, str(e), e.response.status_code) + raise + except Exception as e: + logger.error("Unexpected error downloading from Nextcloud: %s", str(e)) + raise async def get_direct_url(self, path: str) -> str: return self._dav_url(path)