diff --git a/api/src/rehearsalhub/routers/internal.py b/api/src/rehearsalhub/routers/internal.py index dd1448c..92065b3 100644 --- a/api/src/rehearsalhub/routers/internal.py +++ b/api/src/rehearsalhub/routers/internal.py @@ -53,12 +53,17 @@ async def nc_upload( log.warning("nc-upload: band slug '%s' not found in DB", band_slug) return {"status": "skipped", "reason": "band not found"} - # Determine song title and folder from remaining path segments - # e.g. bands/my-band/songs/session1/rec.mp3 → folder=bands/my-band/songs/session1/, title=session1 - # e.g. bands/my-band/rec.mp3 → folder=bands/my-band/, title=rec + # Determine song title and folder from path. + # The title is always the filename stem (e.g. "take1" from "take1.wav"). + # The nc_folder groups all versions of the same recording (the parent directory). + # + # Examples: + # bands/my-band/take1.wav → folder=bands/my-band/, title=take1 + # bands/my-band/231015/take1.wav → folder=bands/my-band/231015/, title=take1 + # bands/my-band/songs/groove/take1.wav → folder=bands/my-band/songs/groove/, title=take1 parent = str(Path(path).parent) nc_folder = parent.rstrip("/") + "/" - title = Path(path).stem if len(parts) == 3 else parts[-2] + title = Path(path).stem version_repo = AudioVersionRepository(session) if event.nc_file_etag and await version_repo.get_by_etag(event.nc_file_etag): diff --git a/api/src/rehearsalhub/routers/songs.py b/api/src/rehearsalhub/routers/songs.py index 8b95bf2..1363f70 100644 --- a/api/src/rehearsalhub/routers/songs.py +++ b/api/src/rehearsalhub/routers/songs.py @@ -3,6 +3,7 @@ import uuid from pathlib import Path from fastapi import APIRouter, Depends, HTTPException, status +from pydantic import BaseModel from sqlalchemy.ext.asyncio import AsyncSession from rehearsalhub.db.engine import get_session @@ -25,6 +26,14 @@ router = APIRouter(tags=["songs"]) AUDIO_EXTENSIONS = {".mp3", ".wav", ".flac", ".ogg", ".m4a", ".aac", ".opus"} +class NcScanResult(BaseModel): + folder: str + files_found: int + imported: int + skipped: int + songs: list[SongRead] + + @router.get("/bands/{band_id}/songs", response_model=list[SongRead]) async def list_songs( band_id: uuid.UUID, @@ -65,7 +74,7 @@ async def create_song( return read -@router.post("/bands/{band_id}/nc-scan", response_model=list[SongRead]) +@router.post("/bands/{band_id}/nc-scan", response_model=NcScanResult) async def scan_nextcloud( band_id: uuid.UUID, session: AsyncSession = Depends(get_session), @@ -98,7 +107,8 @@ async def scan_nextcloud( return href[len(dav_prefix):] return href.lstrip("/") - imported: list[SongRead] = [] + imported_songs: list[SongRead] = [] + skipped_count = 0 band_folder = band.nc_folder_path or f"bands/{band.slug}/" log.info("Starting NC scan for band '%s' in folder '%s'", band.slug, band_folder) @@ -159,6 +169,7 @@ async def scan_nextcloud( if etag and await version_repo.get_by_etag(etag): log.debug("Skipping '%s' — etag already registered", nc_file_path) + skipped_count += 1 continue # Find or create song record @@ -191,12 +202,21 @@ async def scan_nextcloud( read = SongRead.model_validate(song) read.version_count = 1 - imported.append(read) + imported_songs.append(read) label_info = f" [rehearsal: {rehearsal_label}]" if rehearsal_label else "" log.info("Imported '%s' as song '%s'%s", nc_file_path, song_title, label_info) - log.info("NC scan complete: %d new versions imported", len(imported)) - return imported + log.info( + "NC scan complete for '%s': %d imported, %d skipped (already registered)", + band_folder, len(imported_songs), skipped_count, + ) + return NcScanResult( + folder=band_folder, + files_found=len(to_import), + imported=len(imported_songs), + skipped=skipped_count, + songs=imported_songs, + ) # ── Comments ────────────────────────────────────────────────────────────────── diff --git a/watcher/src/watcher/config.py b/watcher/src/watcher/config.py index a8c8e6b..20b004b 100644 --- a/watcher/src/watcher/config.py +++ b/watcher/src/watcher/config.py @@ -16,7 +16,7 @@ class WatcherSettings(BaseSettings): poll_interval: int = 30 # seconds # File extensions to watch - audio_extensions: list[str] = [".wav", ".mp3", ".flac", ".aac", ".ogg", ".m4a"] + audio_extensions: list[str] = [".wav", ".mp3", ".flac", ".aac", ".ogg", ".m4a", ".opus"] @lru_cache diff --git a/watcher/src/watcher/event_loop.py b/watcher/src/watcher/event_loop.py index 1e7267b..abfad58 100644 --- a/watcher/src/watcher/event_loop.py +++ b/watcher/src/watcher/event_loop.py @@ -16,11 +16,15 @@ log = logging.getLogger("watcher.event_loop") # Persist last seen activity ID in-process (good enough for a POC) _last_activity_id: int = 0 -# Nextcloud activity app subject names for file create/modify events. -# These differ across NC versions; we accept all known variants. +# Nextcloud Activity API v2 filter sets. +# +# NC 22+ returns: type="file_created"|"file_changed" (subject is human-readable) +# NC <22 returns: type="files" (subject is a machine key like "created_self") +# +# We accept either style so the watcher works across NC versions. +_UPLOAD_TYPES = {"file_created", "file_changed"} + _UPLOAD_SUBJECTS = { - "file_created", - "file_changed", "created_by", "changed_by", "created_public", @@ -110,11 +114,11 @@ async def poll_once(nc_client: NextcloudWatcherClient, settings: WatcherSettings for activity in activities: activity_id = int(activity.get("activity_id", 0)) activity_type = activity.get("type", "") - subject = activity.get("subject", "") # human-readable, for logging only + subject = activity.get("subject", "") raw_path = extract_nc_file_path(activity) log.debug( - "Activity id=%d type=%r subject=%r path=%r", + "Activity id=%d type=%r subject=%r raw_path=%r", activity_id, activity_type, subject, raw_path, ) @@ -142,8 +146,11 @@ async def poll_once(nc_client: NextcloudWatcherClient, settings: WatcherSettings ) continue - if activity_type not in _UPLOAD_SUBJECTS: - log.debug("Skipping activity %d: type %r not a file upload event", activity_id, activity_type) + if activity_type not in _UPLOAD_TYPES and subject not in _UPLOAD_SUBJECTS: + log.debug( + "Skipping activity %d: type=%r subject=%r — not a file upload event", + activity_id, activity_type, subject, + ) continue log.info("Detected audio upload: %s (activity %d)", nc_path, activity_id) diff --git a/watcher/tests/test_event_loop.py b/watcher/tests/test_event_loop.py index 15fe1eb..e14ab00 100644 --- a/watcher/tests/test_event_loop.py +++ b/watcher/tests/test_event_loop.py @@ -8,6 +8,7 @@ from watcher.event_loop import ( extract_nc_file_path, is_audio_file, is_band_audio_path, + normalize_nc_path, poll_once, ) @@ -27,6 +28,22 @@ def test_is_band_audio_path(): assert not is_band_audio_path("/") +def test_normalize_nc_path_strips_dav_prefix(): + assert normalize_nc_path("remote.php/dav/files/ncadmin/bands/myband/take.wav", "ncadmin") == "bands/myband/take.wav" + + +def test_normalize_nc_path_strips_user_files_prefix(): + assert normalize_nc_path("ncadmin/files/bands/myband/take.wav", "ncadmin") == "bands/myband/take.wav" + + +def test_normalize_nc_path_strips_files_prefix(): + assert normalize_nc_path("files/bands/myband/take.wav", "ncadmin") == "bands/myband/take.wav" + + +def test_normalize_nc_path_already_relative(): + assert normalize_nc_path("bands/myband/take.wav", "ncadmin") == "bands/myband/take.wav" + + def test_extract_nc_file_path_from_objects(): activity = {"objects": {"42": "/bands/foo/songs/bar/take.wav"}} path = extract_nc_file_path(activity) @@ -45,16 +62,25 @@ def test_extract_nc_file_path_returns_none_when_missing(): assert path is None +def _make_nc_mock(username: str = "admin") -> AsyncMock: + """Return a properly configured NextcloudWatcherClient mock.""" + from watcher.nc_client import NextcloudWatcherClient + nc = AsyncMock(spec=NextcloudWatcherClient) + nc.username = username + return nc + + +# ── poll_once: NC 22+ style (type field) ────────────────────────────────────── + @pytest.mark.asyncio async def test_poll_once_ignores_non_audio_files(settings): - from watcher.nc_client import NextcloudWatcherClient - - nc = AsyncMock(spec=NextcloudWatcherClient) + nc = _make_nc_mock() nc.get_activities.return_value = [ { "activity_id": 1, - "subject": "file_created", - "objects": {"1": "/bands/foo/songs/bar/image.jpg"}, + "type": "file_created", + "subject": "created_self", + "objects": {"1": "bands/foo/songs/bar/image.jpg"}, } ] @@ -64,15 +90,14 @@ async def test_poll_once_ignores_non_audio_files(settings): @pytest.mark.asyncio -async def test_poll_once_registers_audio_upload(settings): - from watcher.nc_client import NextcloudWatcherClient - - nc = AsyncMock(spec=NextcloudWatcherClient) +async def test_poll_once_registers_audio_upload_new_nc_style(settings): + nc = _make_nc_mock() nc.get_activities.return_value = [ { "activity_id": 5, - "subject": "file_created", - "objects": {"10": "/bands/myband/songs/mysong/take1.wav"}, + "type": "file_created", + "subject": "created_self", + "objects": {"10": "bands/myband/songs/mysong/take1.wav"}, } ] nc.get_file_etag.return_value = "abc123" @@ -82,22 +107,63 @@ async def test_poll_once_registers_audio_upload(settings): ) as mock_register: await poll_once(nc, settings) mock_register.assert_called_once_with( - "/bands/myband/songs/mysong/take1.wav", + "bands/myband/songs/mysong/take1.wav", "abc123", settings.api_url, ) @pytest.mark.asyncio -async def test_poll_once_ignores_non_file_events(settings): - from watcher.nc_client import NextcloudWatcherClient +async def test_poll_once_registers_audio_upload_old_nc_style(settings): + """Old NC: type='files', subject='created_self' (machine key).""" + nc = _make_nc_mock() + nc.get_activities.return_value = [ + { + "activity_id": 7, + "type": "files", + "subject": "created_self", + "objects": {"11": "bands/myband/231015/take1.wav"}, + } + ] + nc.get_file_etag.return_value = "etag999" - nc = AsyncMock(spec=NextcloudWatcherClient) + with patch( + "watcher.event_loop.register_version_with_api", new_callable=AsyncMock, return_value=True + ) as mock_register: + await poll_once(nc, settings) + mock_register.assert_called_once_with( + "bands/myband/231015/take1.wav", + "etag999", + settings.api_url, + ) + + +@pytest.mark.asyncio +async def test_poll_once_ignores_non_file_events(settings): + nc = _make_nc_mock() nc.get_activities.return_value = [ { "activity_id": 2, - "subject": "shared", # not file_created or file_changed - "objects": {"5": "/bands/foo/songs/bar/take.wav"}, + "type": "shared_with_user", + "subject": "shared_with_you", + "objects": {"5": "bands/foo/songs/bar/take.wav"}, + } + ] + + with patch("watcher.event_loop.register_version_with_api") as mock_register: + await poll_once(nc, settings) + mock_register.assert_not_called() + + +@pytest.mark.asyncio +async def test_poll_once_ignores_non_band_audio_files(settings): + nc = _make_nc_mock() + nc.get_activities.return_value = [ + { + "activity_id": 3, + "type": "file_created", + "subject": "created_self", + "objects": {"6": "Music/personal/track.mp3"}, } ] @@ -108,11 +174,36 @@ async def test_poll_once_ignores_non_file_events(settings): @pytest.mark.asyncio async def test_poll_once_empty_activities_does_nothing(settings): - from watcher.nc_client import NextcloudWatcherClient - - nc = AsyncMock(spec=NextcloudWatcherClient) + nc = _make_nc_mock() nc.get_activities.return_value = [] with patch("watcher.event_loop.register_version_with_api") as mock_register: await poll_once(nc, settings) mock_register.assert_not_called() + + +@pytest.mark.asyncio +async def test_poll_once_advances_cursor(settings): + nc = _make_nc_mock() + nc.get_activities.return_value = [ + { + "activity_id": 10, + "type": "file_created", + "subject": "created_self", + "objects": {"1": "bands/band/cover.jpg"}, # non-audio → skipped + }, + { + "activity_id": 20, + "type": "file_created", + "subject": "created_self", + "objects": {"2": "bands/band/doc.pdf"}, # non-audio → skipped + }, + ] + + import watcher.event_loop as el + el._last_activity_id = 0 + with patch("watcher.event_loop.register_version_with_api"): + await poll_once(nc, settings) + + # Cursor should advance to the highest seen activity_id + assert el._last_activity_id == 20 diff --git a/web/src/pages/BandPage.tsx b/web/src/pages/BandPage.tsx index d6a9650..d016d68 100644 --- a/web/src/pages/BandPage.tsx +++ b/web/src/pages/BandPage.tsx @@ -26,6 +26,14 @@ interface BandInvite { expires_at: string; } +interface NcScanResult { + folder: string; + files_found: number; + imported: number; + skipped: number; + songs: SongSummary[]; +} + export function BandPage() { const { bandId } = useParams<{ bandId: string }>(); const qc = useQueryClient(); @@ -67,15 +75,17 @@ export function BandPage() { }); const scanMutation = useMutation({ - mutationFn: () => api.post(`/bands/${bandId}/nc-scan`, {}), - onSuccess: (imported) => { + mutationFn: () => api.post(`/bands/${bandId}/nc-scan`, {}), + onSuccess: (result) => { qc.invalidateQueries({ queryKey: ["songs", bandId] }); - setScanMsg( - imported.length > 0 - ? `Imported ${imported.length} new song${imported.length !== 1 ? "s" : ""} from Nextcloud.` - : "No new audio files found in Nextcloud." - ); - setTimeout(() => setScanMsg(null), 4000); + if (result.imported > 0) { + setScanMsg(`Imported ${result.imported} new song${result.imported !== 1 ? "s" : ""} from ${result.folder} (${result.skipped} already registered).`); + } else if (result.files_found === 0) { + setScanMsg(`No audio files found in ${result.folder}.`); + } else { + setScanMsg(`All ${result.files_found} file${result.files_found !== 1 ? "s" : ""} in ${result.folder} already registered.`); + } + setTimeout(() => setScanMsg(null), 6000); }, onError: (err) => setScanMsg(err instanceof Error ? err.message : "Scan failed"), });