diff --git a/app/api/jobs.py b/app/api/jobs.py index bb7a3ae3..eecca686 100644 --- a/app/api/jobs.py +++ b/app/api/jobs.py @@ -16,7 +16,7 @@ from fastapi import APIRouter, HTTPException, Request from fastapi.responses import JSONResponse, Response -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, Field, StrictBool, field_validator from app.core.config import ( DISCOGS_LOOKUP_BUDGET_SEC, @@ -44,6 +44,7 @@ from app.core.registry import persist as registry_persist from app.core.registry import register_if_capacity as registry_register_if_capacity from app.core.registry import remove as registry_remove +from app.core.registry import set_favorite as registry_set_favorite from app.core.registry import set_trashed as registry_set_trashed from app.core.settings import ( get_acoustid_api_key, @@ -68,6 +69,7 @@ copy_lyrics, find_lyrics, keep_answer, + lookup_lyrics, lyrics_path, lyrics_settled, read_candidates, @@ -423,6 +425,25 @@ def restore_job(job_id: str) -> dict: return {"job_id": job.id, "trashed_at": job.trashed_at} +class FavoriteBody(BaseModel): + """Strict, so a string such as "false" is refused rather than read as true.""" + + favorite: StrictBool + + +@router.put("/{job_id}/favorite") +def set_favorite(job_id: str, body: FavoriteBody) -> dict: + """Mark a job as a favourite or take it back out (#734). Kept on the server + so the desktop and the phone share one answer.""" + if not JOB_ID_RE.match(job_id): + raise HTTPException(status_code=404, detail="job not found") + job = registry_set_favorite(job_id, body.favorite) + if job is None: + raise HTTPException(status_code=404, detail="job not found") + registry_persist(JOBS_DIR) + return {"job_id": job.id, "favorite": job.favorite} + + @router.get("/{job_id}") def get_job(job_id: str) -> dict: """Get the current state of a job by ID.""" @@ -1174,6 +1195,84 @@ def get_lyrics(job_id: str) -> Response: raise HTTPException(status_code=404, detail="no lyrics") +class LookupBand(BaseModel): + """The band saved on the track from the artist box, which lives in the + studio's own store and may not be on the job yet.""" + + id: str = Field(pattern=r"^Q\d{1,12}$") + name: str = Field(default="", max_length=300) + englishName: str = Field(default="", max_length=300) # noqa: N815 (the store's own key) + + +class LyricsLookupBody(BaseModel): + band: LookupBand | None = None + + +# Jobs whose lyrics the tab is looking up now, so a second request for the +# same track waits for the first rather than asking LRCLIB twice. +_LYRICS_LOOKUPS: set[str] = set() + + +@router.post("/{job_id}/lyrics/lookup") +async def lookup_lyrics_route(job_id: str, body: LyricsLookupBody | None = None) -> Response: + """Look the track's lyrics up on LRCLIB now, for the Lyrics tab (#719). + + The tab used to search LRCLIB itself, with its own copy of the rules that + decide which version is this song by this artist and which length fits. + The two copies had to be kept equal by hand, and when they drifted the + same track could get lyrics at import and none in the tab. This runs the + import's own lookup instead, keeps what it finds the way the import does, + and answers exactly as GET .../lyrics would afterwards. + + Only ever asked when the tab is opened on a track with no lyrics, or the + user presses Look up again: LRCLIB is never reached on its own. 502 when + LRCLIB cannot be reached, so the tab can offer to try again; 404 with + {"nothing_known": true} when the track says too little to look up. + """ + job, job_dir = _lyrics_job(job_id) + if await asyncio.to_thread(lyrics_path(job_dir).is_file): + return await asyncio.to_thread(get_lyrics, job_id) + if job_id in _LYRICS_LOOKUPS: + raise HTTPException(status_code=409, detail="already looking") + band = body.band.model_dump() if body and body.band else None + query = build_query(job, band=band) + if query is None: + return JSONResponse( + {"detail": "nothing to look up", "others": [], "nothing_known": True}, status_code=404 + ) + _LYRICS_LOOKUPS.add(job_id) + try: + answer = await asyncio.wait_for( + asyncio.to_thread( + lookup_lyrics, + query, + cancelled=lambda: registry_get(job_id) is not job, + fallback_title=job.title or "", + ), + timeout=LYRICS_LOOKUP_BUDGET_SEC + TIMEOUT_LYRICS_LOOKUP + 1, + ) + except asyncio.TimeoutError: + logger.info("[%s] lyrics lookup timed out", job_id) + raise HTTPException(status_code=502, detail="lyrics service unreachable") from None + except Exception: + logger.info("[%s] lyrics lookup failed", job_id, exc_info=True) + raise HTTPException(status_code=502, detail="lyrics service unreachable") from None + finally: + _LYRICS_LOOKUPS.discard(job_id) + if registry_get(job_id) is not job: + raise HTTPException(status_code=404, detail="job not found") + if ( + answer is not None + and not lyrics_path(job_dir).is_file() + and await asyncio.to_thread(keep_answer, job, job_dir, answer) + ): + registry_persist(JOBS_DIR) + try: + return await asyncio.to_thread(get_lyrics, job_id) + except HTTPException: + return JSONResponse({"detail": "no lyrics", "others": []}, status_code=404) + + class LyricsOffsetBody(BaseModel): """How many seconds later than their own timing the lyrics are shown: negative for earlier, 0 for their own timing again.""" diff --git a/app/core/models.py b/app/core/models.py index 77e4abc9..e51c84d2 100644 --- a/app/core/models.py +++ b/app/core/models.py @@ -306,6 +306,16 @@ class Job: # A timestamp rather than a bool so the Trash can say when, and so a future # auto-purge has something to work from. trashed_at: float | None = None + # Whether the user marked this track as a favourite, or None if no + # client has said either way yet (#734). + # + # Server-side for the same reason as trashed_at: favourites lived only in + # the desktop page's catalog store, so the phone, which builds its library + # from GET /api/jobs, had no way to set one or to filter by them. None + # rather than False lets the desktop tell "never recorded here" from + # "taken back out", so it can hand up favourites it set before this field + # existed without undoing one removed on the phone. + favorite: bool | None = None # Set by POST /api/jobs/{id}/cancel; consumed by pipeline stages. # Not surfaced via to_state() -- it's internal control state. cancel_requested: bool = False @@ -366,6 +376,7 @@ def to_state(self) -> dict[str, Any]: "stage_timings": self.stage_timings, "vocal_split": self.vocal_split, "trashed_at": self.trashed_at, + "favorite": self.favorite, "created_at": self.created_at, } diff --git a/app/core/registry.py b/app/core/registry.py index 1a732c3e..9b16793f 100644 --- a/app/core/registry.py +++ b/app/core/registry.py @@ -107,6 +107,16 @@ def set_trashed(job_id: str, trashed: bool) -> Job | None: return job +def set_favorite(job_id: str, favorite: bool) -> Job | None: + """Mark a job as a favourite, or take it back out (#734).""" + with _lock: + job = _jobs.get(job_id) + if job is None: + return None + job.favorite = favorite + return job + + def all_jobs() -> dict[str, Job]: """Return a snapshot of the registry for sweep / cleanup.""" with _lock: diff --git a/app/main.py b/app/main.py index 4d5fde4f..238d576b 100644 --- a/app/main.py +++ b/app/main.py @@ -924,10 +924,11 @@ def download_logs_zip() -> StreamingResponse: # it does not re-enable JS eval, new Function or string timers. 'unsafe-inline' is allowed for *styles* # only (the UI sets many style attributes). Allowances: # connect-src -> same-origin API/SSE, the GitHub update check, Tauri IPC, -# Wikidata/Wikipedia for the artist box the now-playing card -# opens, and LRCLIB for the Lyrics tab (#699). All four are -# read-only public APIs, asked only about the open track, and -# sent only an artist and a song name. +# and Wikidata/Wikipedia for the artist box the now-playing +# card opens. Read-only public APIs, asked only about the +# open track, and sent only an artist and a song name. Not +# LRCLIB: the Lyrics tab asks the server to look lyrics up +# (#719), so the page never reaches it. # img-src https: -> remote YouTube/SoundCloud thumbnails # style/font -> the Google Fonts _CSP = ( @@ -942,8 +943,7 @@ def download_logs_zip() -> StreamingResponse: # (#186). They are inline/same-origin schemes, not network endpoints, so # they add no exfiltration channel — script-src below stays locked. "connect-src 'self' https://api.github.com ipc: http://ipc.localhost data: blob: " - "https://www.wikidata.org https://query.wikidata.org https://*.wikipedia.org " - "https://lrclib.net; " + "https://www.wikidata.org https://query.wikidata.org https://*.wikipedia.org; " "object-src 'none'; base-uri 'self'; frame-ancestors 'none'; form-action 'self'" ) diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index b6a073b8..0a8becdf 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -103,6 +103,12 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "base64" +version = "0.23.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5" + [[package]] name = "bit-set" version = "0.8.0" @@ -2503,11 +2509,11 @@ checksum = "19f132c84eca552bf34cab8ec81f1c1dcc229b811638f9d283dceabe58c5569e" [[package]] name = "plist" -version = "1.9.0" +version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "092791278e026273c1b65bbdcfbba3a300f2994c896bd01ab01da613c29c46f1" +checksum = "2896bade328c13f7042a297ea5ac5b0951f6cf989dea5f32c2fd98da398195cb" dependencies = [ - "base64 0.22.1", + "base64 0.23.1", "indexmap 2.14.0", "quick-xml", "serde", @@ -2635,9 +2641,9 @@ dependencies = [ [[package]] name = "quick-xml" -version = "0.39.3" +version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "721da970c312655cde9b4ffe0547f20a8494866a4af5ff51f18b7c633d0c870b" +checksum = "41b1177fdf999d2321d3fb46ff47159d9c1fb9ad66a4879f8c50a0b504615e9b" dependencies = [ "memchr", ] @@ -2990,9 +2996,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.40" +version = "0.23.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef86cd5876211988985292b91c96a8f2d298df24e75989a43a3c73f2d4d8168b" +checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" dependencies = [ "once_cell", "ring", @@ -3014,9 +3020,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.13" +version = "0.103.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" +checksum = "f3c3cf1d8b1e7d4927e2d154c3fcb02979afb9939629c62cd9048d4f07b60ac2" dependencies = [ "ring", "rustls-pki-types", diff --git a/static/css/daw.css b/static/css/daw.css index 4ae32ca0..b4527f1f 100644 --- a/static/css/daw.css +++ b/static/css/daw.css @@ -292,7 +292,7 @@ input, textarea { font-family: inherit; } color: var(--color); } /* Automatic song-structure detection. It sits at the end of the composer next - to Split stems and matches its footprint, so the pair reads as two segments + to Extract stems and matches its footprint, so the pair reads as two segments of one bar rather than a chip loose beside a button. Colour still separates them: the timeline blue, never the amber the primary action owns. */ .structure-toggle { @@ -4840,19 +4840,27 @@ input, textarea { font-family: inherit; } and are therefore exactly the same width. As a flex row the search grew to whatever was left over, which is why it ran past the last chip. - Columns: content | detect | split | rule | card. Row one is the import - gesture, row two is what to extract. Split sits straight after the options - it acts on, and the loaded track comes after the rule, at the end: in the - middle of the row it read as Split's input, when it is only what is - playing (#699). */ + Columns: content | actions | panel toggles | rule | card. Row one is the + import gesture, row two is what to extract. The two actions stack in one + column, Detect structure over Extract stems, so Extract sits on the chips' + row, straight after the options it acts on, and the Collapse toggles stack + beside them (#725). Stacked rather than side by side, the right-hand end + is about 50px narrower, which is room the card keeps. The loaded track + comes after the rule, at the end: in the middle of the row it read as + Extract's input, when it is only what is playing (#699). */ height: auto; display: grid; /* minmax(0, 1fr), not 1fr: a grid track's automatic minimum is its min-content, so a plain 1fr refuses to shrink below the widest thing in - it. The card sat at its natural width and pushed Split stems off the right + it. The card sat at its natural width and pushed Extract stems off the right edge of its own container. Zero as the floor lets the column give way, and the card's own text clips with an ellipsis rather than forcing the row. */ - grid-template-columns: minmax(0, max-content) auto auto auto minmax(0, 1fr); + /* The card's floor is its compact form, the heart and the About this song + button and nothing else (#735). Below what the whole card needs, the + chips fold into their overflow button first (fitStemChips), so the card + is never squeezed out of the row the way display: none once took it. */ + --np-min: 72px; + grid-template-columns: minmax(0, max-content) auto auto auto minmax(var(--np-min), 1fr); align-items: center; row-gap: 8px; column-gap: 10px; @@ -5003,8 +5011,13 @@ input, textarea { font-family: inherit; } border-left: none; } .daw-composer > .daw-process-btn { - grid-column: 3; - grid-row: 1; + grid-column: 2; + grid-row: 2; + /* As wide as Detect structure above it, and as tall as the chips beside + it, so the bar keeps its height. */ + justify-self: stretch; + min-height: 30px; + height: 30px; } /* 2. Ten percent off the waveform: REMOVED. @@ -5024,31 +5037,30 @@ input, textarea { font-family: inherit; } and it would need the strip and the footer inset to match, or something given the freed space. */ -/* Row two, under the two actions: on the chips' line rather than below it. +/* The Collapse toggles, a column of their own beside the two actions (#725): + the label on top, then one toggle per panel, spanning both rows. Beside the + actions rather than under them, which is what lets Extract stems take the + chips' row. - Columns 2 to 4, the two action columns only, not the rule's: a spanning - item's minimum lands in the first track it covers, and the 1px rule column - would have been handed the toggles' whole width. The action columns are - already as wide as Detect structure and Split stems, which is more than the - toggles need, so spanning only those costs the row nothing. - - A grid item rather than absolutely positioned: out of flow it sat on the - right line and would have landed on top of the Extract chips the moment a - window was narrow enough for the two to meet. */ + A grid item rather than absolutely positioned: out of flow it would land on + top of the Extract chips the moment a window was narrow enough for the two + to meet. */ .daw-composer > .daw-panel-toggles { - grid-column: 2 / 4; - grid-row: 2; - justify-self: end; + grid-column: 3; + grid-row: 1 / 3; align-self: center; + flex-direction: column; + align-items: stretch; + justify-content: center; width: auto; min-width: 0; margin: 0; - /* The wrapper stops being a box of its own: each toggle is a chip now, so a + /* The wrapper stops being a box of its own: each toggle is a chip, so a frame around the group would be a box inside a box. */ padding: 0; background: none; border: none; - gap: 6px; + gap: 4px; } /* 4. The toggles become chips, the same size as the Extract ones. @@ -5057,7 +5069,10 @@ input, textarea { font-family: inherit; } The colours stay theirs, including the strike-through that means a panel is put away. */ .daw-composer-stack .daw-panel-toggle { - height: 30px; + /* 26, not the chips' 30: three to a column have to fit in the height of + the two rows beside them. */ + height: 26px; + justify-content: center; padding: 0 11px; border: 1px solid var(--border); border-radius: 6px; @@ -5075,7 +5090,8 @@ input, textarea { font-family: inherit; } font-size: 12px; letter-spacing: -0.005em; text-transform: none; - margin-right: 4px; + text-align: center; + margin: 0; padding: 0; border: none; } @@ -5130,7 +5146,7 @@ input, textarea { font-family: inherit; } align-self: stretch; /* .footer-track-slot is `width: var(--daw-col-w)`, which is the gutter width it was built to fill in the footer. Here the column decides the width, so - the fixed one has to go or the card never shrinks and pushes Split stems + the fixed one has to go or the card never shrinks and pushes Extract stems out of the composer on a narrow window. */ width: auto; min-width: 0; @@ -5459,25 +5475,25 @@ input, textarea { font-family: inherit; } display: none; } -/* Below about 1450px the two rows cannot hold everything. - * - * Measured rather than picked: the chips need 655px at their widest, the - * toggles 280, Detect structure 164 and Split stems 140. With gaps and the - * bar's own padding that is 1299px of composer before the card gets a pixel, - * which is a window of roughly 1450 once the rail and the margins are paid - * for. - * - * So the two things that were moved up here give way, in the order they can be - * done without: the card first, since the track is also named in the library - * and the mixer, then the toggles drop to a line of their own, which is where - * they lived before. The card's favourite heart is also on every library row - * (#724). Its About this song button has no second home yet (#735). - */ -@media (max-width: 1460px) { - .daw-composer > #nowPlayingPanel { display: none; } - /* The rule only divides the extraction from the card. Without the card it - would divide the row from nothing. */ - .daw-composer > .daw-composer-sep { display: none; } +/* A narrow card keeps what can be done from it (#735). + + It used to leave the bar entirely below a 1460px window, and with it the + only way to open About this song. Now it gives way in steps as its column + narrows, which happens after the Extract chips have folded what they can: + first the artwork and the meta line, then the title, until only the heart + and the About button are left, each still a full-height segment at its + edge. Container queries, because how wide the card ends up depends on the + language as much as on the window. */ +@container (max-width: 239px) { + #nowPlayingPanel .track-panel-cover, + #nowPlayingPanel .daw-track-meta-line, + #nowPlayingPanel .np-artist { display: none; } + #nowPlayingPanel .track-panel-head { padding-left: calc(var(--np-fav) + 2px); } + #nowPlayingPanel .np-legend { left: calc(var(--np-fav) + 8px); } +} +@container (max-width: 129px) { + #nowPlayingPanel .track-panel-info, + #nowPlayingPanel .np-legend { display: none; } } /* The click-track options with nothing to act on. diff --git a/static/css/variables.css b/static/css/variables.css index e81c5cad..b5ae03f3 100644 --- a/static/css/variables.css +++ b/static/css/variables.css @@ -26,7 +26,7 @@ --other: #9ca3af; /* Width shared by the composer's two end segments (Song structure and - Split stems) so they match. See .daw-process-btn / .structure-toggle. */ + Extract stems) so they match. See .daw-process-btn / .structure-toggle. */ --composer-action-w: 140px; /* Accent */ diff --git a/static/index.html b/static/index.html index 1a7182b1..a69fa64a 100644 --- a/static/index.html +++ b/static/index.html @@ -270,7 +270,7 @@ - Split stems + Extract stems diff --git a/static/js/catalog.js b/static/js/catalog.js index ef8d6d42..8fae4f6a 100644 --- a/static/js/catalog.js +++ b/static/js/catalog.js @@ -942,6 +942,41 @@ function syncTrashToServer(trackId, trashed) { .catch((e) => console.warn(`[catalog] could not ${action} ${trackId} on the server`, e)); } +/** + * Tell the server a track was favourited or taken back out (#734). + * + * The server holds the answer the phone reads, and the phone can change it, so + * this side adopts the server's value on every sync (applyServerFavorite) and + * only says something when the user presses a heart here. Not awaited, for the + * same reason as the Trash: a heart must answer at once. + */ +function syncFavoriteToServer(trackId, favorite) { + fetch(`/api/jobs/${encodeURIComponent(trackId)}/favorite`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ favorite }), + }).catch((e) => console.warn(`[catalog] could not save the favourite for ${trackId}`, e)); +} + +/** + * Take the server's favourite for a track this side already knows. Returns + * whether anything changed here. + * + * null on the server means no client has said either way, which is every + * track from before favourites moved there. A heart set here then is handed up + * rather than dropped; anything the server does know wins, because the phone + * may have changed it since. + */ +function applyServerFavorite(track, state) { + if (state.favorite == null) { + if (track.favorite) syncFavoriteToServer(state.job_id, true); + return false; + } + if (Boolean(track.favorite) === state.favorite) return false; + track.favorite = state.favorite; + return true; +} + function moveTrackToTrash(trackId) { if (!tracks[trackId]) return; removeTrackFromFolders(trackId); @@ -1056,7 +1091,7 @@ async function loadTrackIntoStudio(trackId) { applyStoredStemSelection(track); setCurrentTrack(trackId); - // The composer is an input the Split stems button submits, not a caption for + // The composer is an input the Extract stems button submits, not a caption for // the open track, so it may only ever hold something that can actually be // imported. A `local:` source is a file that was uploaded once and is not // reachable again; putting its bare filename here armed the button with a @@ -1079,7 +1114,7 @@ async function loadTrackIntoStudio(trackId) { // `required` is the guard for "the button has nothing to act on", and it // is a blunt one: the browser refuses the submit before any handler runs, // so it must not be set while the button has a track to re-split. Setting - // it there made pressing Split stems on an upload answer "please fill out + // it there made pressing Extract stems on an upload answer "please fill out // this field" instead of separating it. if (importable || resplittable) urlInput.removeAttribute("required"); else urlInput.setAttribute("required", ""); @@ -1098,7 +1133,7 @@ async function loadTrackIntoStudio(trackId) { } /** - * Aim the Split stems button at a track rather than at the composer. + * Aim the Extract stems button at a track rather than at the composer. * * Carried on the button itself rather than in a module variable so job.js can * read it at submit time without the two files having to agree on an import @@ -1429,6 +1464,7 @@ export function toggleFavorite(trackId) { const track = tracks[trackId]; if (!track) return; track.favorite = !track.favorite; + syncFavoriteToServer(trackId, track.favorite); saveState(); if (trackId === _currentTrackId) { const favBtn = document.getElementById("fav-btn"); @@ -3463,6 +3499,7 @@ async function syncWithServer() { for (const state of jobs) { const known = tracks[state.job_id]; if (known) { + if (applyServerFavorite(known, state)) backfilled = true; // Tracks saved before the server reported a format (#690) only learn // it when they are opened. Taking it from here instead means the // whole library shows its icons at startup. @@ -3492,11 +3529,15 @@ async function syncWithServer() { if (deletedIds.has(state.job_id)) continue; // hard-deleted, skip const track = stateMetadataToTrack(state, { id: state.job_id, status: state.status }); track.id = state.job_id; + if (state.favorite) track.favorite = true; addTrackToLibrary(track); } if (backfilled) { saveState(); render(); + const favBtn = document.getElementById("fav-btn"); + const current = tracks[_currentTrackId]; + if (favBtn && current) paintFavButton(favBtn, Boolean(current.favorite)); } reconcileAvailability(jobs); } catch (e) { console.warn("[catalog] failed to load jobs from backend:", e); } @@ -4956,4 +4997,13 @@ export async function initCatalog() { loadCurrentVersion().finally(checkForUpdate); syncWithServer(); + // A heart pressed on the phone (#734) shows here the next time this window + // is looked at, rather than only after a reload. At most every 15 seconds, + // since switching windows back and forth is not new information. + let lastFocusSync = Date.now(); + document.addEventListener("visibilitychange", () => { + if (document.visibilityState !== "visible" || Date.now() - lastFocusSync < 15_000) return; + lastFocusSync = Date.now(); + syncWithServer(); + }); } diff --git a/static/js/i18n.js b/static/js/i18n.js index 848b1b51..174f2289 100644 --- a/static/js/i18n.js +++ b/static/js/i18n.js @@ -394,7 +394,7 @@ const en = { "lyrics.sync.laneHint": "Drag a line · ← → nudge", "extract.all": "All", "extract.moreAria": "Choose stems", - "process.splitStems": "Split stems", + "process.extractStems": "Extract stems", "stem.original": "Original", "stem.vocals": "Vocals", @@ -583,6 +583,7 @@ const en = { "pitch.downTitle": "Transpose down a semitone", "pitch.upTitle": "Transpose up a semitone", "speed.ariaLabel": "Playback speed", + "speed.slowTitle": "Slow practice speed. Scroll over it, or press the up and down arrow keys, to set it anywhere from 0.50x to 0.99x.", "click.group": "Click track", "click.toggleTitle": "Click track (K)", @@ -1200,7 +1201,7 @@ const pl = { "lyrics.sync.laneHint": "Przeciągnij wers · ← → przesuń", "extract.all": "Wszystko", "extract.moreAria": "Wybierz ścieżki", - "process.splitStems": "Rozdziel ścieżki", + "process.extractStems": "Wyodrębnij ścieżki", "stem.original": "Oryginał", "stem.vocals": "Wokal", @@ -1387,6 +1388,7 @@ const pl = { "pitch.downTitle": "Transponuj w dół o półton", "pitch.upTitle": "Transponuj w górę o półton", "speed.ariaLabel": "Prędkość odtwarzania", + "speed.slowTitle": "Wolne tempo do ćwiczeń. Przewiń nad nim kółkiem myszy albo naciśnij strzałki w górę i w dół, aby ustawić je od 0.50x do 0.99x.", "click.group": "Metronom", "click.toggleTitle": "Metronom (K)", @@ -1996,7 +1998,7 @@ const ja = { "lyrics.sync.laneHint": "行をドラッグ · ← → で微調整", "extract.all": "すべて", "extract.moreAria": "抽出するパートを選ぶ", - "process.splitStems": "パートを分離", + "process.extractStems": "パートを抽出", "stem.original": "オリジナル", "stem.vocals": "ボーカル", @@ -2182,6 +2184,7 @@ const ja = { "pitch.downTitle": "半音下げる", "pitch.upTitle": "半音上げる", "speed.ariaLabel": "再生速度", + "speed.slowTitle": "練習用のスロー速度。上でスクロールするか、上下の矢印キーを押すと、0.50x から 0.99x の間で設定できます。", "click.group": "クリックトラック", "click.toggleTitle": "クリックトラック (K)", @@ -2767,7 +2770,7 @@ const zhHans = { "lyrics.sync.laneHint": "拖动歌词行 · ← → 微调", "extract.all": "全部", "extract.moreAria": "选择要提取的音轨", - "process.splitStems": "分离音轨", + "process.extractStems": "提取音轨", "stem.original": "原始", "stem.vocals": "人声", @@ -2953,6 +2956,7 @@ const zhHans = { "pitch.downTitle": "降低半音", "pitch.upTitle": "升高半音", "speed.ariaLabel": "播放速度", + "speed.slowTitle": "慢速练习速度。在上面滚动鼠标滚轮，或按上下方向键，可设置为 0.50x 到 0.99x 之间的任意值。", "click.group": "节拍器", "click.toggleTitle": "节拍器 (K)", @@ -3538,7 +3542,7 @@ const de = { "lyrics.sync.laneHint": "Zeile ziehen · ← → verschieben", "extract.all": "Alle", "extract.moreAria": "Stems auswählen", - "process.splitStems": "Stems trennen", + "process.extractStems": "Stems extrahieren", "stem.original": "Original", "stem.vocals": "Gesang", @@ -3725,6 +3729,7 @@ const de = { "pitch.downTitle": "Einen Halbton tiefer transponieren", "pitch.upTitle": "Einen Halbton höher transponieren", "speed.ariaLabel": "Wiedergabegeschwindigkeit", + "speed.slowTitle": "Langsames Übungstempo. Darüber scrollen oder die Pfeiltasten nach oben und unten drücken, um es zwischen 0.50x und 0.99x einzustellen.", "click.group": "Click-Track", "click.toggleTitle": "Click-Track (K)", @@ -4320,7 +4325,7 @@ const pt = { "lyrics.sync.laneHint": "Arraste uma linha · ← → ajustar", "extract.all": "Todos", "extract.moreAria": "Escolher as faixas", - "process.splitStems": "Separar stems", + "process.extractStems": "Extrair stems", "stem.original": "Original", "stem.vocals": "Vocais", @@ -4507,6 +4512,7 @@ const pt = { "pitch.downTitle": "Transpor um semitom para baixo", "pitch.upTitle": "Transpor um semitom para cima", "speed.ariaLabel": "Velocidade de reprodução", + "speed.slowTitle": "Velocidade lenta para praticar. Role o mouse sobre ela ou pressione as setas para cima e para baixo para ajustá-la entre 0.50x e 0.99x.", "click.group": "Clique de referência", "click.toggleTitle": "Clique de referência (K)", @@ -5104,7 +5110,7 @@ const id = { "lyrics.sync.laneHint": "Seret baris · ← → geser", "extract.all": "Semua", "extract.moreAria": "Pilih stem", - "process.splitStems": "Pisahkan stem", + "process.extractStems": "Ekstrak stem", "stem.original": "Asli", "stem.vocals": "Vokal", @@ -5290,6 +5296,7 @@ const id = { "pitch.downTitle": "Turunkan satu semitone", "pitch.upTitle": "Naikkan satu semitone", "speed.ariaLabel": "Kecepatan putar", + "speed.slowTitle": "Kecepatan lambat untuk latihan. Gulir di atasnya, atau tekan tombol panah atas dan bawah, untuk mengaturnya antara 0.50x dan 0.99x.", "click.group": "Click Track", "click.toggleTitle": "Click track (K)", @@ -5875,7 +5882,7 @@ const fr = { "lyrics.sync.laneHint": "Glissez une ligne · ← → ajuster", "extract.all": "Tout", "extract.moreAria": "Choisir les pistes", - "process.splitStems": "Séparer les pistes", + "process.extractStems": "Extraire les pistes", "stem.original": "Original", "stem.vocals": "Voix", @@ -6062,6 +6069,7 @@ const fr = { "pitch.downTitle": "Transposer d'un demi-ton vers le bas", "pitch.upTitle": "Transposer d'un demi-ton vers le haut", "speed.ariaLabel": "Vitesse de lecture", + "speed.slowTitle": "Vitesse lente d’entraînement. Faites défiler dessus, ou appuyez sur les flèches haut et bas, pour la régler entre 0.50x et 0.99x.", "click.group": "Métronome", "click.toggleTitle": "Métronome (K)", @@ -6486,6 +6494,7 @@ const fr = { // through pt (see FALLBACK), so the two variants cannot drift and a key // added to pt later is picked up here rather than reverting to English. const ptPT = { + "speed.slowTitle": "Velocidade lenta para praticar. Desloque a roda do rato sobre ela ou prima as setas para cima e para baixo para a ajustar entre 0.50x e 0.99x.", "key.mode.harmonicMinor": "menor harmónica", "lyrics.align.hint": "Coloque o cursor de reprodução onde a primeira linha é cantada e prima Começar a letra aqui. Para usar outra linha, clique nela primeiro e depois leve o cursor até onde é cantada.", "lyrics.align.targetPicked": "Move a linha em que clicou: “{line}”", @@ -6830,7 +6839,7 @@ const es = { "lyrics.sync.laneHint": "Arrastra una línea · ← → mover", "extract.all": "Todo", "extract.moreAria": "Elegir las pistas", - "process.splitStems": "Separar stems", + "process.extractStems": "Extraer stems", "stem.original": "Original", "stem.vocals": "Voces", @@ -7017,6 +7026,7 @@ const es = { "pitch.downTitle": "Transponer un semitono hacia abajo", "pitch.upTitle": "Transponer un semitono hacia arriba", "speed.ariaLabel": "Velocidad", + "speed.slowTitle": "Velocidad lenta para practicar. Desplázate sobre ella, o pulsa las flechas arriba y abajo, para ajustarla entre 0.50x y 0.99x.", "click.group": "Click", "click.toggleTitle": "Click (K)", @@ -7635,7 +7645,7 @@ const ko = { "lyrics.sync.laneHint": "줄 끌기 · ← → 옮기기", "extract.all": "전체", "extract.moreAria": "추출할 스템 선택", - "process.splitStems": "스템 분리", + "process.extractStems": "스템 추출", "stem.original": "원본", "stem.vocals": "보컬", @@ -7821,6 +7831,7 @@ const ko = { "pitch.downTitle": "반음 내리기", "pitch.upTitle": "반음 올리기", "speed.ariaLabel": "재생 속도", + "speed.slowTitle": "연습용 느린 속도입니다. 위에서 스크롤하거나 위아래 화살표 키를 눌러 0.50x에서 0.99x 사이로 설정할 수 있습니다.", "click.group": "클릭 트랙", "click.toggleTitle": "클릭 트랙 (K)", diff --git a/static/js/job.js b/static/js/job.js index 547b39e7..e6bd9e53 100644 --- a/static/js/job.js +++ b/static/js/job.js @@ -113,11 +113,11 @@ function setSubmitProcessing(processing) { const label = submitBtn.querySelector("span"); // Back to the label the markup ships, not a second one that only exists // here. This restored t("job.process") -- "Process" -- while index.html - // starts the button as t("process.splitStems") -- "Split stems" -- so the + // starts the button as t("process.extractStems") -- "Extract stems" -- so the // first submit of a session renamed the button for good, on success as much // as on failure, and nothing put it back short of a reload or a language // switch (#635). - if (label) label.textContent = processing ? t("job.processing") : t("process.splitStems"); + if (label) label.textContent = processing ? t("job.processing") : t("process.extractStems"); } /** True when audio is loaded in the studio. Either engine counts: the Web Audio diff --git a/static/js/lyrics.js b/static/js/lyrics.js index 55395ca9..eb21275e 100644 --- a/static/js/lyrics.js +++ b/static/js/lyrics.js @@ -43,15 +43,11 @@ import { getCurrentTrackInfo, getCurrentTrackArtist } from "./catalog.js"; import { transport, setPlayheadTime } from "./transport.js"; import { totalDuration } from "./state.js"; import { - searchLyrics, - rankVersions, - otherNames, parseLrc, currentLineIndex, wordTimings, songFromTitle, fromServerLyrics, - belongsTo, sameSong, clampOffset, shiftLines, @@ -82,11 +78,6 @@ import { // change to any track. const storeKey = (trackId) => `stemdeck.lyrics.${trackId}`; -// Within this many seconds of the track's length, an automatic search keeps a -// version without asking. LRCLIB lengths come from real releases, so the same -// recording lands within a second or two; a live cut or a radio edit does not. -const SAME_RECORDING_SEC = 3; - const reducedMotion = window.matchMedia("(prefers-reduced-motion: reduce)"); let statusEl = null; @@ -1308,56 +1299,51 @@ function lookAgainButton() { return again; } -// How many of the artist's other names are searched by when its own finds no -// version the track's length. -const OTHER_NAME_SEARCHES = 2; - -const sameLength = (match, info) => Boolean(match) && info.duration > 0 && match.duration > 0 - && Math.abs(match.duration - info.duration) <= SAME_RECORDING_SEC; - /** - * Look the open track up on LRCLIB by what it is known to be. Only versions of - * this song by this artist count (belongsTo): LRCLIB's search also answers with - * other artists' songs, and no lyrics beat another song's. `names` are the - * other names the artist goes by (the band's, in its own script and in - * English), which count as its own, and which are searched by too when the - * artist's name finds no version the track's length: LRCLIB files a song under - * whichever name its uploader wrote, 周杰倫 or Jay Chou. One the same length as - * the track is kept straight away; otherwise they are offered to pick from; - * with none, it says so. What it found is remembered for the session, so - * opening the tab again does not ask again. A failed connection is not, so the - * next opening tries again. + * Ask the server to look the open track up on LRCLIB (#719). + * + * The tab used to search LRCLIB itself, with its own copy of the rules that + * decide which version is this song by this artist and which length fits. + * The two copies were kept equal by hand, and a track could get lyrics at + * import and none here. Now there is one copy, the import's, and what it + * finds is kept on the server like the import's answer, so the phone sees it + * too. The band saved from the artist box goes along: it lives in this side's + * store, and the server may not have it yet. + * + * What came back without lyrics is remembered for the session, so opening the + * tab again does not ask again. A failed connection is not, so the next + * opening tries again. */ -async function lookUp(info, { artist, song, names = [] }) { +async function lookUp(info, { song }) { searchController?.abort(); const controller = new AbortController(); searchController = controller; setStatus(t("lyrics.loading", { song }), "loading"); try { - const known = { artist, song, names }; - const search = async (by) => (await searchLyrics({ artist: by, song, duration: info.duration }, { signal: controller.signal })) - .filter((m) => belongsTo(m, known)); - let matches = await search(artist); - for (const other of otherNames(artist, names).slice(0, OTHER_NAME_SEARCHES)) { - if (sameLength(matches[0], info) || controller.signal.aborted) break; - // A failure here keeps what the artist's own name found. - const more = await search(other).catch((err) => { - console.warn("lyrics lookup by another name failed", err); - return []; - }); - matches = rankVersions([...matches, ...more], info.duration); - } + const band = getCurrentTrackArtist(); + const res = await fetch(`/api/jobs/${encodeURIComponent(info.id)}/lyrics/lookup`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(band ? { band } : {}), + signal: controller.signal, + }); if (controller.signal.aborted || getCurrentTrackInfo()?.id !== info.id) return; - const best = matches[0]; - if (sameLength(best, info)) { - await keep(best, matches.slice(1, 12)); + if (!res.ok && res.status !== 404) throw new Error(`lyrics lookup -> ${res.status}`); + const data = await res.json().catch(() => null); + if (res.status === 404 && data?.nothing_known) { + setStatus(t("lyrics.nothingKnown"), "muted"); + nothingKnownFor = info.id; + return; + } + const found = fromServerLyrics(data); + if (found?.entry) { + show(found.entry, found.others, "server"); return; } - answered.set(info.id, { song, matches: matches.slice(0, 12) }); + answered.set(info.id, { song, matches: found?.others || [] }); showAnswer(answered.get(info.id)); } catch (err) { - // Aborted only by another track being opened; a timeout aborts the - // lookup's own inner signal and lands below, as a failure. + // Aborted only by another track being opened. if (controller.signal.aborted) return; console.warn("lyrics lookup failed", err); setStatus(t("lyrics.offline"), "error"); @@ -1522,7 +1508,7 @@ async function loadForCurrentTrack() { showAnswer(answered.get(info.id)); return; } - lookUp(info, { artist, song, names }); + lookUp(info, { song }); } // While the tab is on screen: notice a different track being opened, mark the diff --git a/static/js/lyricsLookup.js b/static/js/lyricsLookup.js index 9f1a71c9..7fb477f9 100644 --- a/static/js/lyricsLookup.js +++ b/static/js/lyricsLookup.js @@ -1,48 +1,18 @@ -// Lyrics for the open track, from LRCLIB (lrclib.net), for the Lyrics tab in -// the sidebar (#699). +// Lyrics for the Lyrics tab (#699): what the server found, read into the +// tab's shape, and the helpers that parse and time them. // -// LRCLIB because it is free, open and needs no key, and because most of what -// it holds is time-synced: each line carries the moment it is sung, so the tab -// can follow playback and a line can be clicked to jump there, which is what -// practising against the stems wants. The lyrics themselves are copyright of -// their writers; they are fetched when the Lyrics tab is opened on a track -// that has none kept, and kept on this machine for that track, never sent on. +// Finding them is the server's job alone (POST /api/jobs/{id}/lyrics/lookup, +// app/pipeline/lyrics_lookup.py). The tab used to search LRCLIB itself with +// its own copy of the rules for which version is this song by this artist, +// and the two copies drifted (#719). What is left here is the reading and the +// timing, which the tab and Sync lines need either way, and sameSong, which +// checks lyrics this tab kept before that rule existed. // -// What is sent is the artist and song the track is known to be (its tags, or -// the saved band and the song's name from the title), nothing else. -// -// No DOM here, and fetch is passed in, so this runs under node for the unit -// tests (tests/js/lyrics-lookup.test.mjs). +// No DOM here, so this runs under node for the unit tests. import { artistNameKey } from "./artistLookup.js"; import { toSimplified } from "./zhVariants.js"; -const LRCLIB_SEARCH = "https://lrclib.net/api/search"; -const TIMEOUT_MS = 12000; - -// Among versions this far apart or closer, prefer the one with synced lyrics. -// LRCLIB holds the same song many times over (live cuts, remasters, a rip with -// a second of silence trimmed), and a synced copy a second off beats a plain -// one that matches to the frame. -const SAME_LENGTH_SEC = 3; - -/** - * One LRCLIB row, reduced to what the tab uses. Rows with neither kind of - * lyrics and no instrumental flag say nothing, and are dropped by the caller. - */ -function normalise(row) { - return { - id: Number(row?.id) || 0, - track: String(row?.trackName || ""), - artist: String(row?.artistName || ""), - album: String(row?.albumName || ""), - duration: Number(row?.duration) || 0, - instrumental: Boolean(row?.instrumental), - synced: typeof row?.syncedLyrics === "string" ? row.syncedLyrics : "", - plain: typeof row?.plainLyrics === "string" ? row.plainLyrics : "", - }; -} - const SERVER_SOURCES = new Set(["lrclib", "file", "whisper"]); /** One version from the server's lyrics.json in the tab's shape, or null. */ @@ -113,107 +83,6 @@ export function fromServerLyrics(data) { return others.length ? { entry: null, others } : null; } -/** - * Best first, for a track `duration` seconds long (0 when unknown): - * closest in length, then synced over plain among versions within - * SAME_LENGTH_SEC of each other, then intact over a copy stripped of its - * accents (strippedCopy), then whatever LRCLIB ranked first. - */ -export function rankMatches(rows, duration = 0) { - const matches = (Array.isArray(rows) ? rows : []) - .map(normalise) - .filter((m) => m.id && (m.synced || m.plain || m.instrumental)); - return rankVersions(matches, duration); -} - -/** rankMatches' order over versions it already gave, say from two searches. - * Each version once, by id. */ -export function rankVersions(versions, duration = 0) { - const seen = new Set(); - const matches = versions.filter((m) => m?.id && !seen.has(m.id) && seen.add(m.id)); - const off = (m) => (duration && m.duration ? Math.abs(m.duration - duration) : 0); - const stripped = strippedIndexes(matches); - return matches - .map((m, rank) => ({ m, rank })) - .sort((a, b) => { - const da = off(a.m); - const db = off(b.m); - if (Math.abs(da - db) > SAME_LENGTH_SEC) return da - db; - if (Boolean(a.m.synced) !== Boolean(b.m.synced)) return a.m.synced ? -1 : 1; - const sa = stripped.has(a.rank); - if (sa !== stripped.has(b.rank)) return sa ? 1 : -1; - return da - db || a.rank - b.rank; - }) - .map(({ m }) => m); -} - -// Stripped copies, as _Words in app/pipeline/lyrics_lookup.py. LRCLIB holds -// many songs more than once, and some copies lost every letter outside ASCII -// on their way in: "Niewinnoci biaym niegiem" for "Niewinnością białym -// śniegiem" (Kayah, lrclib 5470091 beside the intact 10910419). Each such -// letter was either dropped or folded to its base ("się" as "sie"). Such a copy -// cannot be told from a song written without accents on its own, only beside -// the copy it was stripped from. -const LRC_TAGS = /\[[^\]\n]*\]|<\d{1,3}:\d{1,2}(?:[.:]\d{1,3})?>/g; -const NOT_ASCII = /[^\p{ASCII}]/gu; -const isAscii = (word) => !/[^\p{ASCII}]/u.test(word); -// The intact copy has at least this many words with a letter outside ASCII, -// so a stray "café" proves nothing; the stripped one keeps at most a quarter -// of them; at least 60% of the intact copy's accented words appear in it with -// those letters dropped or folded; at least 80% of its words are the intact -// copy's. -const STRIPPED_MIN_WORDS = 5; -const STRIPPED_KEPT_MAX = 0.25; -const STRIPPED_FOUND_MIN = 0.6; -const STRIPPED_SAME_MIN = 0.8; - -/** What an accented word becomes with its letters outside ASCII dropped - * ("każe" as "kae") or folded to their base first ("się" as "sie"). */ -function strippedForms(word) { - const forms = [word.replace(NOT_ASCII, ""), word.normalize("NFD").replace(NOT_ASCII, "")]; - return forms.filter(Boolean); -} - -/** A version's words, lowercased, time stamps left out, with what they would - * be stripped: worked out once per version, compared many times. */ -function wordsOf(match) { - const text = String(match?.synced || match?.plain || "").normalize("NFC").replace(LRC_TAGS, " "); - const words = text.toLowerCase().match(/[\p{L}\p{M}]+/gu) || []; - const accented = words.filter((w) => !isAscii(w)); - const forms = accented.map(strippedForms); - return { words, have: new Set(words), accented, forms, known: new Set([...words, ...forms.flat()]) }; -} - -function strippedFrom(copy, intact) { - const accented = intact.accented.length; - if (accented < STRIPPED_MIN_WORDS || !copy.words.length) return false; - if (copy.accented.length > accented * STRIPPED_KEPT_MAX) return false; - const found = intact.forms.filter((forms) => forms.some((f) => copy.have.has(f))).length; - if (found < accented * STRIPPED_FOUND_MIN) return false; - const same = copy.words.filter((w) => intact.known.has(w)).length; - return same >= copy.words.length * STRIPPED_SAME_MIN; -} - -/** - * Whether `match` is `other`'s lyrics with the letters outside ASCII lost, - * dropped or folded to their base letter. Both are versions as rankMatches - * gives them ({ synced, plain }). - */ -export function strippedCopy(match, other) { - return strippedFrom(wordsOf(match), wordsOf(other)); -} - -/** The indexes in `matches` of versions that are a stripped copy of another. */ -function strippedIndexes(matches) { - const words = matches.map(wordsOf); - const intact = words.map((w, j) => [j, w]).filter(([, w]) => w.accented.length >= STRIPPED_MIN_WORDS); - const out = new Set(); - words.forEach((w, i) => { - if (intact.some(([j, other]) => j !== i && strippedFrom(w, other))) out.add(i); - }); - return out; -} - /** * Timed lines from LRC text, earliest first: [{ time, text }], time in * seconds. A line sung more than once carries several stamps @@ -615,31 +484,10 @@ export function songFromTitle(title, artist = "") { .trim(); } -// Whose song a version is: belongs_to in app/pipeline/lyrics_lookup.py. -// -// LRCLIB's search is fuzzy: asked for one artist's song it also answers with -// other artists' songs of a similar name. Lyrics that may be another song's are -// worse than none, so a version is kept only when its song is the one asked for -// and its artist is one of the names the track is known by: the artist itself, -// one of the artists credited with it ("Keala Settle" of "Keala Settle & The -// Greatest Showman Ensemble"), the show a cast recording is filed under, or -// another name the band goes by (its name in the artist box). Names are -// compared folded (fold): full-width letters as half-width, traditional -// Chinese as simplified. -const NAME_LIST = /\s*(?:[&,+/;]|\band\b|\bwith\b|\bfeat\.?|\bft\.?|\bfeaturing\b|\bvs\.?)\s*/iu; -const LEADING_THE = /^\s*the\s+/iu; +// What sameSong compares: a song's name less brackets and featured artists. const SONG_FEATURING = /\s+(?:ft\.?|feat\.?|featuring)\s.*$/iu; const SONG_BRACKETS = /[([{【][^()[\]{}【】]*[)\]}】]/gu; const SONG_TAIL = /\s+\p{Pd}+\s+.*$/u; -// What stands between the names a name is written in at once: "周杰倫 Jay -// Chou", "IU (아이유)", "五月天 (Mayday)". -const SCRIPT_BREAK = /[\s()[\]{}【】「」『』〈〉《》]+/u; -// One credited artist matches only when its name weighs at least this much -// (nameWeight: an ideograph counts two), so an initial or a stray "DJ" names -// nobody while 王菲 does; a run of words inside a longer name ("The Greatest -// Showman" in "The Greatest Showman Cast") needs this many. -const PART_MIN_CHARS = 4; -const RUN_MIN_WORDS = 2; // Latin letters Unicode does not build from a base letter and an accent, as a // name typed without them has them: "Podsiadlo" for Podsiadło. @@ -653,85 +501,6 @@ export function fold(text) { return toSimplified(String(text || "").normalize("NFKC").replace(LATIN_LETTER, (ch) => LATIN_LETTERS[ch])); } -const isHan = (cp) => - (cp >= 0x3400 && cp <= 0x4dbf) || (cp >= 0x4e00 && cp <= 0x9fff) || (cp >= 0xf900 && cp <= 0xfaff) || (cp >= 0x20000 && cp <= 0x3ffff); - -/** A Chinese, Japanese or Korean letter: an ideograph, kana or Hangul. */ -function isCjk(ch) { - const cp = ch.codePointAt(0); - return isHan(cp) - || (cp >= 0x3040 && cp <= 0x30ff) || (cp >= 0x31f0 && cp <= 0x31ff) || (cp >= 0xff66 && cp <= 0xff9f) - || (cp >= 0x1100 && cp <= 0x11ff) || (cp >= 0x3130 && cp <= 0x318f) || (cp >= 0xac00 && cp <= 0xd7af); -} - -/** How much of a name a name key is, in Latin letters: name_weight in - * name_aliases.py. An ideograph counts two, marks nothing. */ -function nameWeight(key) { - let weight = 0; - for (const ch of key) weight += isHan(ch.codePointAt(0)) ? 2 : /\p{M}/u.test(ch) ? 0 : 1; - return weight; -} - -const nameKey = (name) => artistNameKey(fold(name).replace(LEADING_THE, "")); -const nameWords = (name) => (fold(name).match(/[\p{L}\p{M}\p{N}]+/gu) || []).map(artistNameKey).filter(Boolean); -const nameParts = (name) => fold(name).split(NAME_LIST).map(nameKey).filter(Boolean); - -function scriptOf(token) { - const letters = token.match(/\p{L}/gu) || []; - if (!letters.length) return ""; - const cjk = letters.filter(isCjk).length; - return cjk === letters.length ? "cjk" : cjk ? "mixed" : "other"; -} - -/** - * The names a name gives in two scripts at once, each on its own: "周杰倫 Jay - * Chou" as 周杰倫 and "Jay Chou", "IU (아이유)" as IU and 아이유. [] for a name - * in one script, or with a word that mixes them ("Official髭男dism"). - * script_names in app/pipeline/lyrics_lookup.py. - */ -export function scriptNames(name) { - const tokens = fold(name).split(SCRIPT_BREAK).filter(Boolean); - const kinds = tokens.map(scriptOf); - if (kinds.includes("mixed") || new Set(kinds.filter(Boolean)).size < 2) return []; - const groups = []; - let last = ""; - tokens.forEach((token, i) => { - if (!kinds[i]) { - last = ""; - return; - } - if (kinds[i] !== last) groups.push([]); - groups.at(-1).push(token); - last = kinds[i]; - }); - return groups.map((group) => group.join(" ")); -} - -const wholeNames = (name) => [nameKey(name), ...scriptNames(name).map(nameKey)].filter(Boolean); - -function containsRun(words, run) { - if (run.length < RUN_MIN_WORDS || run.length > words.length) return false; - for (let i = 0; i + run.length <= words.length; i++) { - if (run.every((w, j) => words[i + j] === w)) return true; - } - return false; -} - -/** Whether `found`, a version's artist, is one of `names`. */ -export function sameArtist(found, names) { - if (!nameKey(found)) return false; - const theirs = new Set(wholeNames(found)); - const theirParts = nameParts(found); - const theirWords = nameWords(found); - return names.some((name) => { - if (!String(name || "").trim()) return false; - if (wholeNames(name).some((key) => theirs.has(key))) return true; - if (nameParts(name).some((p) => nameWeight(p) >= PART_MIN_CHARS && theirParts.includes(p))) return true; - const mine = nameWords(name); - return containsRun(mine, theirWords) || containsRun(theirWords, mine); - }); -} - function songKeys(name) { const text = fold(name); let full = text.replace(SONG_BRACKETS, " ").replace(SONG_FEATURING, ""); @@ -753,31 +522,6 @@ export function sameSong(found, song) { return a.full === b.full || a.head === b.full || a.full === b.head; } -/** Whether a version is `song` by `artist` or one of `names` (a show's, the - * band's other names). */ -export function belongsTo(match, { artist = "", song = "", names = [] } = {}) { - return sameSong(match?.track, song) && sameArtist(match?.artist, [artist, ...names]); -} - -/** - * Who else to search LRCLIB by when the artist's own name found no version - * the track's length: the names it gives at once ("周杰倫 Jay Chou"), then - * `names` (the band's in the artist box), each once and never the artist's - * own. An uploader files a song under whichever one they write. - */ -export function otherNames(artist, names = []) { - const asked = new Set([String(artist || "").trim().toLowerCase()]); - const out = []; - for (const name of [...scriptNames(artist), ...names]) { - const text = String(name || "").trim(); - if (text && !asked.has(text.toLowerCase())) { - asked.add(text.toLowerCase()); - out.push(text); - } - } - return out; -} - // How far the Align panel moves lyrics either way, as the server bounds it // (LYRICS_OFFSET_MAX_SEC in app/core/config.py). export const MAX_OFFSET_SEC = 600; @@ -834,43 +578,3 @@ export function currentLineIndex(lines, seconds) { } return found; } - -/** The request for a search, as a URL. The song is required, the artist not. */ -export function searchUrl({ artist = "", song = "" }) { - const url = new URL(LRCLIB_SEARCH); - const a = String(artist).trim(); - const s = String(song).trim(); - if (a) { - url.searchParams.set("track_name", s); - url.searchParams.set("artist_name", a); - } else { - // No artist: the free-text search, which matches the song name against - // titles and artists both, rather than an empty artist filter. - url.searchParams.set("q", s); - } - return url.toString(); -} - -async function defaultFetchJson(url, signal) { - const res = await fetch(url, { signal, headers: { Accept: "application/json" } }); - if (!res.ok) throw new Error(`HTTP ${res.status} from ${new URL(url).host}`); - return res.json(); -} - -/** - * Search LRCLIB and rank what comes back for a track `duration` seconds long. - * Resolves to [] when nothing matches, and rejects when LRCLIB cannot be - * reached, so the tab can tell the two apart. - */ -export async function searchLyrics({ artist = "", song = "", duration = 0 }, { fetchJson = defaultFetchJson, signal } = {}) { - if (!String(song).trim()) return []; - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(new Error("timeout")), TIMEOUT_MS); - signal?.addEventListener("abort", () => controller.abort(signal.reason), { once: true }); - try { - const rows = await fetchJson(searchUrl({ artist, song }), controller.signal); - return rankMatches(rows, duration); - } finally { - clearTimeout(timer); - } -} diff --git a/static/js/shared/jobs.js b/static/js/shared/jobs.js index 7a4edd60..6b74247a 100644 --- a/static/js/shared/jobs.js +++ b/static/js/shared/jobs.js @@ -76,5 +76,8 @@ export function jobToCard(state) { // can be reimported without a second fetch for its own source URL/stems. sourceUrl: state.source_url || null, selectedStems: state.selected_stems || [], + // Kept on the server since #734, so a heart set on either screen shows on + // the other. null (never said) reads as not a favourite. + favorite: state.favorite === true, }; } diff --git a/static/js/transport.js b/static/js/transport.js index aa5b18c2..37a5393c 100644 --- a/static/js/transport.js +++ b/static/js/transport.js @@ -962,35 +962,67 @@ export function wireTransportButtons() { wirePitchControl(); } -// Fixed presets, not a continuous dial -- practice speeds for slowing a part -// down, not a general-purpose tempo control (issue #269 follow-up). -// 0.75x rather than 0.5x/0.25x (#433): below ~0.7x the time-stretch artefacts -// dominate and the part gets harder to follow, which is the opposite of what -// a practice speed is for. -const SPEED_PRESETS = [0.75, 1]; +// Two buttons, normal and slow, rather than a dial -- practice speeds for +// slowing a part down, not a general-purpose tempo control (#269 follow-up). +// +// The slow one is set by the player (#701): scroll over it, or press the up +// and down arrow keys on it, to move it a hundredth at a time between +// SLOW_MIN and SLOW_MAX. Some fills want 0.75x, some only a nudge to 0.9x. +// It was fixed at 0.75x when the stretch was WSOLA, whose artefacts made +// anything slower hard to follow (#433); Signalsmith Stretch (#729) holds up +// further down, so the floor is 0.5x. Kept between sessions, and the footer +// keeps its width: the button only ever reads "0.xxx". +const SLOW_MIN = 0.5; +const SLOW_MAX = 0.99; +const SLOW_DEFAULT = 0.75; +const _SLOW_SPEED_KEY = "stemdeck:slow-speed"; +// A turn of the wheel fires many events; the rate itself is applied once +// they stop, since every change of rate flushes the tempo stage. +const SLOW_APPLY_DELAY_MS = 150; + +const clampSlow = (rate) => Math.round(Math.min(SLOW_MAX, Math.max(SLOW_MIN, rate)) * 100) / 100; +let _slowRate = SLOW_DEFAULT; +let _slowApplyTimer = 0; +const slowBtn = () => speedBtns[0]; + +function paintSlowButton() { + const btn = slowBtn(); + if (!btn) return; + btn.dataset.speed = String(_slowRate); + btn.textContent = `${_slowRate.toFixed(2)}x`; + btn.title = t("speed.slowTitle"); +} function applySpeed(rate) { - // Snap to the nearest preset rather than clamping continuously: every - // caller (button click, resetSpeed on track load) already passes one of - // SPEED_PRESETS, but snapping keeps this correct even if that changes. - const clamped = SPEED_PRESETS.reduce((best, p) => - Math.abs(p - rate) < Math.abs(best - rate) ? p : best - ); - setPlaybackSpeed(clamped); + // Normal or the slow speed: nothing in between is offered, so anything + // under 1 is the slow button's. + const applied = rate >= 1 ? 1 : _slowRate; + setPlaybackSpeed(applied); for (const btn of speedBtns) { if (!btn) continue; - const on = parseFloat(btn.dataset.speed) === clamped; + const on = parseFloat(btn.dataset.speed) === applied; btn.classList.toggle("active", on); btn.setAttribute("aria-checked", on ? "true" : "false"); } - audioEngine?.setPlaybackRate?.(clamped); + audioEngine?.setPlaybackRate?.(applied); if (multitrack) { for (const a of (multitrack.audios ?? [])) { - try { a.playbackRate = clamped; } catch { /* noop */ } + try { a.playbackRate = applied; } catch { /* noop */ } } } } +/** Move the slow speed by `steps` hundredths, and play at it. */ +function nudgeSlowSpeed(steps) { + const next = clampSlow(_slowRate + steps / 100); + if (next === _slowRate) return; + _slowRate = next; + paintSlowButton(); + storeSet(_SLOW_SPEED_KEY, _slowRate).catch((e) => console.warn("[transport] failed to save the slow speed:", e)); + clearTimeout(_slowApplyTimer); + _slowApplyTimer = setTimeout(() => applySpeed(_slowRate), SLOW_APPLY_DELAY_MS); +} + // ── Transpose (#245) ──────────────────────────────────────────────────────── // @@ -1092,6 +1124,26 @@ function wireSpeedControl() { for (const btn of speedBtns) { btn?.addEventListener("click", () => applySpeed(parseFloat(btn.dataset.speed))); } + const slow = slowBtn(); + if (!slow) return; + paintSlowButton(); + onLanguageChange(paintSlowButton); + slow.addEventListener("wheel", (e) => { + e.preventDefault(); + if (e.deltaY) nudgeSlowSpeed(e.deltaY < 0 ? 1 : -1); + }, { passive: false }); + slow.addEventListener("keydown", (e) => { + if (e.key !== "ArrowUp" && e.key !== "ArrowDown") return; + e.preventDefault(); + nudgeSlowSpeed(e.key === "ArrowUp" ? 1 : -1); + }); + storeGet(_SLOW_SPEED_KEY, null).then((saved) => { + if (typeof saved !== "number" || !Number.isFinite(saved)) return; + const wasSlow = slow.classList.contains("active"); + _slowRate = clampSlow(saved); + paintSlowButton(); + if (wasSlow) applySpeed(_slowRate); + }).catch(() => {}); } // ─── Click track ──────────────────────────────────────────── diff --git a/static/js/utils.js b/static/js/utils.js index 166b2cad..918b7dd1 100644 --- a/static/js/utils.js +++ b/static/js/utils.js @@ -139,7 +139,7 @@ export function parseTimecode(str) { // Everything else is a link the importer can act on. // // One predicate rather than three spellings of it. The composer arming the -// Split stems button and the re-import of an unavailable track are asking the +// Extract stems button and the re-import of an unavailable track are asking the // same question, and the answer stopped agreeing between them: the composer // showed an uploaded file's bare filename as though it were a link, and the // button then submitted it (#635). diff --git a/static/mobile/app.js b/static/mobile/app.js index d645fa5a..7bcf4f1e 100644 --- a/static/mobile/app.js +++ b/static/mobile/app.js @@ -66,6 +66,7 @@ const ICON = { search: '', link: '', upload: '', + heart: (on) => ``, check: '', scissors: '', tabLib: '', @@ -541,8 +542,15 @@ function libraryBody() { if (state.libState === "empty") { return `
No tracks yet. Head to Extract to split your first song.
`; } - return `
RECENT
- ${state.tracks.map((t) => { + // The Favorites chip used to set a filter nothing read, so it listed every + // track (#734). Favourites are on the server now, so it can filter for real. + const favOnly = state.filter === "Favorites"; + const shown = favOnly ? state.tracks.filter((t) => t.favorite) : state.tracks; + if (favOnly && !shown.length) { + return `
No favourites yet. Tap the heart on a track to add it here.
`; + } + return `
${favOnly ? "FAVORITES" : "RECENT"}
+ ${shown.map((t) => { const unavailable = t.status === "unavailable"; const canReimport = unavailable && t.sourceUrl && !t.sourceUrl.startsWith("local:"); const infoHtml = unavailable @@ -556,6 +564,7 @@ function libraryBody() {
${artLabel(t)}
${infoHtml}
+ `; @@ -620,7 +629,7 @@ function extractScreen() { - + ${extractProgressCard()} `; @@ -841,6 +850,29 @@ function closeSwipe() { closeOtherSwipes(null); } +// The same flag the desktop's hearts set (#734). The row changes at once and +// goes back if the server refuses, so a tap never shows a state that was not +// kept. +async function toggleFavorite(id) { + const track = state.tracks.find((t) => t.id === id); + if (!track) return; + track.favorite = !track.favorite; + render(); + try { + const res = await fetch(`/api/jobs/${encodeURIComponent(id)}/favorite`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ favorite: track.favorite }), + }); + if (!res.ok) throw new Error(`PUT favorite -> ${res.status}`); + } catch (e) { + console.warn("[mobile] could not save the favourite:", e); + track.favorite = !track.favorite; + toast("Could not save the favourite"); + render(); + } +} + async function deleteTrack(id) { state.swipedTrackId = null; try { @@ -962,6 +994,11 @@ app.addEventListener("click", (e) => { case "delete": deleteTrack(t.dataset.id); return; + case "favorite": + // The row underneath opens the track; the heart is its own button. + e.stopPropagation(); + toggleFavorite(t.dataset.id); + return; case "tab": state.tab = t.dataset.tab; break; diff --git a/static/mobile/styles.css b/static/mobile/styles.css index 7cae237f..817fe836 100644 --- a/static/mobile/styles.css +++ b/static/mobile/styles.css @@ -778,6 +778,19 @@ button { opacity: 1; transform: translate(-50%, 0); } +.track-fav { + flex-shrink: 0; + display: grid; + place-items: center; + width: 40px; + height: 40px; + margin-right: 2px; + border: none; + background: none; + color: #7a7a82; + cursor: pointer; +} +.track-fav.on { color: #e54e4e; } .track-load { flex-shrink: 0; border: 1px solid rgba(255, 255, 255, 0.12); diff --git a/tests/acceptance/README.md b/tests/acceptance/README.md index 65a226de..4571f3fb 100644 --- a/tests/acceptance/README.md +++ b/tests/acceptance/README.md @@ -113,7 +113,7 @@ window are added, so the karaoke checks do not depend on the window being on top. The window is resized with the Win32 API, because the now-playing card only shows in a window wider than 1460 px. -E4 blocks `lrclib.net` for the page alone. E6 starts the app a second time +E4 answers the page's lyrics lookup with the server's "LRCLIB unreachable" (502), for the page alone. E6 starts the app a second time with every outside request sent to a proxy that nobody listens on (`HTTP(S)_PROXY` for the backend, `--proxy-server` for WebView2), loopback excepted: offline for this app, while the machine keeps its network. diff --git a/tests/acceptance/acceptance.spec.mjs b/tests/acceptance/acceptance.spec.mjs index 628ed8f2..3cd85f3e 100644 --- a/tests/acceptance/acceptance.spec.mjs +++ b/tests/acceptance/acceptance.spec.mjs @@ -103,10 +103,9 @@ test.afterEach(async ({ app }, testInfo) => { if (testInfo.status === testInfo.expectedStatus) return; await shot(app.page, testInfo, "failure").catch(() => {}); const message = testInfo.error?.message || ""; - const evidence = serviceEvidence(app.page, checkStartedAt, { - // Blocked on purpose by E4. - ignoreHosts: testInfo.title.startsWith("E4") ? ["lrclib.net"] : [], - }); + // E4's refused lookup is a request to the app's own server, which this does + // not count as an outside service. + const evidence = serviceEvidence(app.page, checkStartedAt); // An outage only explains a failure it could have caused: an import that // YouTube refused, or a request from the page itself that failed. A service // in the backend log alone is noted, not blamed. @@ -342,7 +341,7 @@ test(title("A3"), async ({ app }, testInfo) => { // ─── Now playing and song details ─────────────────────────────────────────── // Steps: import any song and open it. Expect, left to right: the link field, -// the Extract options, Split stems, then the Now playing card with the +// the Extract options, Extract stems, then the Now playing card with the // artwork, the heart on its left and the (i) on its right. test(title("N1"), async ({ app }, testInfo) => { const { page } = app; @@ -367,8 +366,8 @@ test(title("N1"), async ({ app }, testInfo) => { // chips under the field; everything else follows them to the right. const besideOrUnder = url.right <= extract.left || (Math.abs(extract.left - url.left) < 40 && extract.top >= url.bottom - 2); expect(besideOrUnder, "the Extract options follow the link field").toBe(true); - expect(Math.max(url.right, extract.right) <= split.left, "Split stems follows them").toBe(true); - expect(split.right <= card.left, "the card comes after Split stems").toBe(true); + expect(Math.max(url.right, extract.right) <= split.left, "Extract stems follows them").toBe(true); + expect(split.right <= card.left, "the card comes after Extract stems").toBe(true); // One row: every one of them overlaps the card's height. for (const b of [url, extract, split]) expect(b.top < card.bottom && b.bottom > card.top, "one row").toBe(true); await expect(page.locator("#nowPlayingPanel .np-legend")).toHaveText("Now playing"); @@ -1021,16 +1020,19 @@ test(title("V2"), async ({ app }, testInfo) => { // Expect: "Could not reach LRCLIB. Check your connection and try again." in // the Lyrics panel; nothing floats over the top bar. // -// LRCLIB is blocked for the page alone (page.route over CDP): the rest of the -// machine keeps its network. The page only asks LRCLIB itself for a track the -// server kept no lyrics for, which the tagged tone file is. +// The page asks the server to look lyrics up (#719), and the server answers +// 502 when LRCLIB cannot be reached. That answer is given for the page alone +// (page.route over CDP), so the rest of the machine keeps its network. The +// lookup is only asked for a track the server kept no lyrics for, which the +// tagged tone file is. test(title("E4"), async ({ app }, testInfo) => { const { page } = app; await wideWindow(page); const id = await trackFor(page, "localMp3"); const kept = await fetch(`${readState().baseURL}/api/jobs/${id}/lyrics`); - test.skip(kept.status !== 404, `The server kept lyrics for the tone file (HTTP ${kept.status}), so the page would not ask LRCLIB.`); - await page.route(/^https:\/\/lrclib\.net\//, (route) => route.abort("internetdisconnected")); + test.skip(kept.status !== 404, `The server kept lyrics for the tone file (HTTP ${kept.status}), so the page would not ask for a lookup.`); + const LOOKUP = /\/api\/jobs\/[^/]+\/lyrics\/lookup$/; + await page.route(LOOKUP, (route) => route.fulfill({ status: 502, json: { detail: "lyrics service unreachable" } })); try { await openTrack(page, id); await openLyrics(page); @@ -1050,7 +1052,7 @@ test(title("E4"), async ({ app }, testInfo) => { expect(floating).toBe(0); await shot(page, testInfo, "offline"); } finally { - await page.unroute(/^https:\/\/lrclib\.net\//); + await page.unroute(LOOKUP); } }); diff --git a/tests/acceptance/helpers.mjs b/tests/acceptance/helpers.mjs index bda75cf7..c52bfdee 100644 --- a/tests/acceptance/helpers.mjs +++ b/tests/acceptance/helpers.mjs @@ -233,7 +233,7 @@ export async function showLibrary(page) { await expect(lib).toHaveAttribute("aria-pressed", "true"); } -/** Paste a link in the top bar's field and press Split stems. Returns the job id. */ +/** Paste a link in the top bar's field and press Extract stems. Returns the job id. */ export async function importLink(page, url) { const field = page.locator("#url"); await field.fill(url); @@ -242,7 +242,7 @@ export async function importLink(page, url) { page.locator("#submit").click(), ]); const body = await res.json().catch(() => ({})); - if (!res.ok()) throw new Error(`Split stems refused ${url}: ${res.status()} ${body.detail || ""}`); + if (!res.ok()) throw new Error(`Extract stems refused ${url}: ${res.status()} ${body.detail || ""}`); return body.job_id; } @@ -255,7 +255,7 @@ export async function importFile(page, file) { page.locator("#submit").click(), ]); const body = await res.json().catch(() => ({})); - if (!res.ok()) throw new Error(`Split stems refused ${path.basename(file)}: ${res.status()} ${body.detail || ""}`); + if (!res.ok()) throw new Error(`Extract stems refused ${path.basename(file)}: ${res.status()} ${body.detail || ""}`); return body.job_id; } @@ -327,7 +327,7 @@ export async function queueImports(page) { /** * The job id of an import, waiting for it to finish. A link YouTube refused * (a 403 on the media now and then) is split once more, as a person would - * press Split stems again; the retry is recorded in the state file so the + * press Extract stems again; the retry is recorded in the state file so the * check can say so. */ export async function songJob(page, key) { diff --git a/tests/e2e/about-reach.spec.mjs b/tests/e2e/about-reach.spec.mjs new file mode 100644 index 00000000..c762974c --- /dev/null +++ b/tests/e2e/about-reach.spec.mjs @@ -0,0 +1,36 @@ +// About this song can be opened at any window width (#735). +// +// Its button lived only on the Now Playing card, and daw.css hid that whole +// card below 1460 px, so a maximised laptop window had no way to open it. The +// card now gives way in steps instead: the artwork and the meta line go first, +// then the title, and the heart and the About button stay. + +import { test, expect } from "@playwright/test"; +import { openStudio } from "./helpers.mjs"; + +for (const width of [1024, 1280, 1366, 1600]) { + test(`About this song opens from the card at ${width} px`, async ({ page }) => { + await page.setViewportSize({ width, height: 800 }); + await openStudio(page); + const button = page.locator("#np-details-btn"); + await expect(button).toBeVisible(); + await expect(page.locator("#fav-btn")).toBeVisible(); + // On screen and inside the bar, not pushed past its edge. + const box = await button.boundingBox(); + const bar = await page.locator(".daw-composer").boundingBox(); + expect(box.width).toBeGreaterThan(20); + expect(box.x + box.width).toBeLessThanOrEqual(bar.x + bar.width + 0.5); + await button.click(); + await expect(page.locator("#artistDialog")).toBeVisible(); + }); +} + +test("Extract stems sits under Detect structure and the composer keeps one line of chips", async ({ page }) => { + await page.setViewportSize({ width: 1600, height: 800 }); + await openStudio(page); + const detect = await page.locator("#autoSectionsBtn").boundingBox(); + const extract = await page.locator("#submit").boundingBox(); + await expect(page.locator("#submit")).toHaveText(/Extract stems/); + expect(Math.abs(detect.x - extract.x)).toBeLessThan(1); + expect(extract.y).toBeGreaterThan(detect.y + detect.height - 1); +}); diff --git a/tests/e2e/audio-tags-backfill.spec.mjs b/tests/e2e/audio-tags-backfill.spec.mjs index 2a9c1d69..74f8b7c2 100644 --- a/tests/e2e/audio-tags-backfill.spec.mjs +++ b/tests/e2e/audio-tags-backfill.spec.mjs @@ -16,13 +16,13 @@ import { stubAudioTags, stubExportEndpoints, stubUpdateCheck, + LYRICS_LOOKUP, } from "./helpers.mjs"; // Wide enough for the now-playing card, which the bar hides below 1460px. test.use({ viewport: { width: 1600, height: 900 } }); const WIKIMEDIA = /^https:\/\/([a-z]+\.wikipedia\.org|www\.wikidata\.org|query\.wikidata\.org)\//; -const LRCLIB = /^https:\/\/lrclib\.net\//; // Longer than artistInfo.js's wait after a track opens. const PAST_THE_WAIT_MS = 2000; @@ -266,15 +266,12 @@ test.describe("tags for tracks imported before they were read", () => { test("the Lyrics tab, open while the tags arrive, looks them up", async ({ page }) => { await stubWikimedia(page); + // The tab asks the server to look them up (#719), which has the tags the + // read just stored; it is answered here with nothing found. const lrclib = []; - await page.route(LRCLIB, (route) => { + await page.route(LYRICS_LOOKUP, (route) => { lrclib.push(route.request().url()); - return route.fulfill({ - status: 200, - contentType: "application/json", - headers: { "access-control-allow-origin": "*" }, - body: "[]", - }); + return route.fulfill({ status: 404, json: { detail: "no lyrics", others: [] } }); }); let release; const hold = new Promise((resolve) => { release = resolve; }); @@ -287,6 +284,6 @@ test.describe("tags for tracks imported before they were read", () => { release(); await expect.poll(() => lrclib.length, { timeout: 15000 }).toBeGreaterThan(0); - expect(decodeURIComponent(lrclib[0]).replace(/\+/g, " ")).toContain("Dream Theater"); + await expect(page.locator("#lyricsStatus")).toContainText("No lyrics found"); }); }); diff --git a/tests/e2e/composer-arming.spec.mjs b/tests/e2e/composer-arming.spec.mjs index a57bbfe5..88b90001 100644 --- a/tests/e2e/composer-arming.spec.mjs +++ b/tests/e2e/composer-arming.spec.mjs @@ -1,4 +1,4 @@ -// What the Split stems button is allowed to be pointed at (#635). +// What the Extract stems button is allowed to be pointed at (#635). // // The composer is an input the button submits, not a caption for whatever // track is open. Opening a track writes its source back into it, and for an diff --git a/tests/e2e/favorite-reach.spec.mjs b/tests/e2e/favorite-reach.spec.mjs index 5cc37dfc..4c1c02d6 100644 --- a/tests/e2e/favorite-reach.spec.mjs +++ b/tests/e2e/favorite-reach.spec.mjs @@ -1,9 +1,10 @@ // A track can be favourited at any window width (#724). // -// The only heart used to be the Now Playing card's, and daw.css hides that -// whole card below 1460 px. A maximised laptop window is under that, so there -// was no way to favourite at all. Library and Favorites rows have one now, and -// every heart goes through one toggle, so they cannot disagree. +// The only heart used to be the Now Playing card's, and daw.css hid that whole +// card below 1460 px. A maximised laptop window is under that, so there was no +// way to favourite at all. Library and Favorites rows have one now, and every +// heart goes through one toggle, so they cannot disagree. The card stays in a +// compact form at narrow widths too since #735, so both hearts are on screen. import { test, expect } from "@playwright/test"; import { JOB_ID, SIBLING_JOB_ID, openStudio, readCatalogState } from "./helpers.mjs"; @@ -15,9 +16,8 @@ const favoriteIn = async (page, id) => Boolean((await readCatalogState(page))?.t test.describe("favourites at a narrow window", () => { test.use({ viewport: { width: 1280, height: 800 } }); - test("a library row can favourite a track the Now Playing card cannot reach", async ({ page }) => { + test("a library row can favourite a track that is not the one playing", async ({ page }) => { await openStudio(page); - await expect(page.locator("#fav-btn")).toBeHidden(); await row(page, SIBLING_JOB_ID).hover(); await expect(rowHeart(page, SIBLING_JOB_ID)).toBeVisible(); diff --git a/tests/e2e/favorite-sync.spec.mjs b/tests/e2e/favorite-sync.spec.mjs new file mode 100644 index 00000000..0b340592 --- /dev/null +++ b/tests/e2e/favorite-sync.spec.mjs @@ -0,0 +1,57 @@ +// Favourites are kept on the server, so the phone and the studio agree (#734). +// +// They lived only in the studio's own store, so the phone had no heart to +// press and its Favorites chip listed every track. The server is stubbed here +// (stubFavorites) so no spec leaves a favourite behind on the shared backend; +// the endpoint itself is covered by tests/test_jobs_favorite.py. + +import { test, expect } from "@playwright/test"; +import { JOB_ID, SIBLING_JOB_ID, openStudio, readCatalogState, stubFavorites } from "./helpers.mjs"; + +const row = (page, id) => page.locator(`#catalogList .cat-item[data-id="${id}"]`).first(); + +test.describe("studio", () => { + test.use({ viewport: { width: 1600, height: 900 } }); + + test("a heart pressed in the studio is saved on the server", async ({ page }) => { + const { favoriteWrites } = await openStudio(page); + await row(page, SIBLING_JOB_ID).hover(); + await row(page, SIBLING_JOB_ID).locator(".cat-fav").click(); + await expect.poll(() => favoriteWrites).toContainEqual({ id: SIBLING_JOB_ID, favorite: true }); + }); + + test("a favourite set on the phone shows in the studio", async ({ page }) => { + await openStudio(page, { serverFavorites: { [SIBLING_JOB_ID]: true } }); + await expect(row(page, SIBLING_JOB_ID)).toHaveClass(/\bis-fav\b/); + expect((await readCatalogState(page))?.tracks?.[SIBLING_JOB_ID]?.favorite).toBe(true); + }); +}); + +test.describe("phone", () => { + test.use({ viewport: { width: 390, height: 844 } }); + + const openPhone = async (page, serverFavorites) => { + const writes = await stubFavorites(page, serverFavorites); + await page.goto("/?ui=mobile", { waitUntil: "domcontentloaded" }); + await page.locator('[data-action="tab"][data-tab="library"]').first().click(); + await page.locator(`.track[data-id="${JOB_ID}"]`).first().waitFor({ timeout: 20000 }); + return writes; + }; + + test("the Favorites chip lists only favourites", async ({ page }) => { + await openPhone(page, { [JOB_ID]: true }); + await page.locator('[data-action="filter"][data-filter="Favorites"]').click(); + await expect(page.locator(`.track[data-id="${JOB_ID}"]`)).toBeVisible(); + await expect(page.locator(`.track[data-id="${SIBLING_JOB_ID}"]`)).toHaveCount(0); + }); + + test("a heart on the phone is saved and does not open the track", async ({ page }) => { + const writes = await openPhone(page, {}); + const heart = page.locator(`.track-fav[data-id="${SIBLING_JOB_ID}"]`); + await heart.click(); + await expect(heart).toHaveAttribute("aria-pressed", "true"); + await expect.poll(() => writes).toContainEqual({ id: SIBLING_JOB_ID, favorite: true }); + // Still on the library, not taken to the player. + await expect(page.locator(`.track[data-id="${SIBLING_JOB_ID}"]`)).toBeVisible(); + }); +}); diff --git a/tests/e2e/helpers.mjs b/tests/e2e/helpers.mjs index 13e19b14..56eb46f3 100644 --- a/tests/e2e/helpers.mjs +++ b/tests/e2e/helpers.mjs @@ -253,8 +253,37 @@ export async function stubUpdateCheck(page, { available = false } = {}) { } /** Open the fixture track in the studio and wait until the transport is live. */ -export async function openStudio(page, { tauri = false, updateAvailable = false } = {}) { +/** + * Keep favourites off the shared e2e backend (#734). + * + * A heart now writes to the server, and every spec shares one server, so a + * favourite set by one test would come back through GET /api/jobs into the + * next one's library. Writes are answered here instead, and the server's + * value stays null, which the desktop reads as "never said". A test about the + * sync itself passes `serverFavorites` to stand in for what the server knows. + */ +export async function stubFavorites(page, serverFavorites = {}) { + const writes = []; + await page.route("**/api/jobs/*/favorite", async (route) => { + const id = new URL(route.request().url()).pathname.split("/")[3]; + const { favorite } = JSON.parse(route.request().postData() || "{}"); + writes.push({ id, favorite }); + serverFavorites[id] = favorite; + await route.fulfill({ json: { job_id: id, favorite } }); + }); + await page.route(/\/api\/jobs(\?.*)?$/, async (route) => { + if (route.request().method() !== "GET") return route.continue(); + const res = await route.fetch(); + const jobs = await res.json(); + for (const j of jobs) j.favorite = serverFavorites[j.job_id] ?? null; + await route.fulfill({ response: res, json: jobs }); + }); + return writes; +} + +export async function openStudio(page, { tauri = false, updateAvailable = false, serverFavorites = {} } = {}) { await seedLibrary(page); + const favoriteWrites = await stubFavorites(page, serverFavorites); if (tauri) await stubTauri(page); await stubExportEndpoints(page); await stubUpdateCheck(page, { available: updateAvailable }); @@ -268,6 +297,7 @@ export async function openStudio(page, { tauri = false, updateAvailable = false null, { timeout: 20000 }, ); + return { favoriteWrites }; } /** @@ -322,3 +352,60 @@ export const exportUi = (page) => ({ await page.locator("#t-export-panel:not(.hidden)").waitFor({ timeout: 5000 }); }, }); + +/** The server's lyrics lookup, which the Lyrics tab asks rather than LRCLIB (#719). */ +export const LYRICS_LOOKUP = /\/api\/jobs\/[a-f0-9]{12}\/lyrics\/lookup$/; + +/** + * Stand in for the server's lyrics lookup, so the lyrics specs run offline and + * never reach LRCLIB through the shared backend. + * + * `rows` are LRCLIB rows. Like the server, it keeps only versions of `song` by + * `artist` (brackets and a " - ..." tail aside), keeps the first within 3 s of + * the track's `duration` and offers the others with it, or offers them all + * when none is that length. What it kept or offered is then what GET + * .../lyrics answers, as the server's lyrics.json would. The rule itself is + * tested against the real server in tests/test_lyrics_lookup.py; this only + * has to answer the way it does. Resolves to the requests the page made: + * [{ id, body }]. + */ +export async function stubLyricsLookup(page, { rows = [], artist = "", song = "", duration = 6, offline = false, nothingKnown = false } = {}) { + const asked = []; + const kept = {}; + const offered = {}; + const jobOf = (route) => new URL(route.request().url()).pathname.split("/")[3]; + const version = (r) => ({ + source: "lrclib", + lrclib_id: r.id, + track: r.trackName, + artist: r.artistName, + album: r.albumName || "", + duration: r.duration, + instrumental: Boolean(r.instrumental), + synced: r.syncedLyrics || "", + plain: r.plainLyrics || "", + }); + const songKey = (s) => String(s || "").replace(/[([{][^)\]}]*[)\]}]/g, "").replace(/\s+-\s+.*$/, "").trim().toLowerCase(); + await page.route(/\/api\/jobs\/[a-f0-9]{12}\/lyrics$/, (route) => { + if (route.request().method() !== "GET") return route.fallback(); + const id = jobOf(route); + if (kept[id]) return route.fulfill({ json: kept[id], headers: { "cache-control": "no-cache" } }); + if (offered[id]?.length) return route.fulfill({ status: 404, json: { detail: "no lyrics", others: offered[id] } }); + return route.fallback(); + }); + await page.route(LYRICS_LOOKUP, (route) => { + const id = jobOf(route); + asked.push({ id, body: JSON.parse(route.request().postData() || "{}") }); + if (offline) return route.fulfill({ status: 502, json: { detail: "lyrics service unreachable" } }); + if (nothingKnown) return route.fulfill({ status: 404, json: { detail: "nothing to look up", others: [], nothing_known: true } }); + const mine = rows.filter((r) => r.artistName === artist && songKey(r.trackName) === songKey(song)).map(version); + const best = mine.find((v) => Math.abs(v.duration - duration) <= 3); + if (best) { + kept[id] = { v: 1, ...best, timing: "exact", others: mine.filter((v) => v !== best) }; + return route.fulfill({ json: kept[id] }); + } + offered[id] = mine; + return route.fulfill({ status: 404, json: { detail: "no lyrics", others: mine } }); + }); + return asked; +} diff --git a/tests/e2e/lyrics-cjk.spec.mjs b/tests/e2e/lyrics-cjk.spec.mjs index c56746bc..9efd5be4 100644 --- a/tests/e2e/lyrics-cjk.spec.mjs +++ b/tests/e2e/lyrics-cjk.spec.mjs @@ -13,8 +13,8 @@ // lyrics-from-server.spec.mjs gives them, and the vocals envelope is absent, so // the timing is the line's own and does not depend on the fixture's audio. // -// Last, the tab's own lookup (no lyrics from the server) finding a song LRCLIB -// files only under the band's name in its own script. +// Last, the tab asking the server to look a track up (no lyrics kept yet), +// with the band saved in both scripts. import { test, expect } from "@playwright/test"; import { JOB_ID, @@ -24,9 +24,10 @@ import { stubAudioTags, stubExportEndpoints, stubUpdateCheck, + stubLyricsLookup, + LYRICS_LOOKUP, } from "./helpers.mjs"; -const LRCLIB = /^https:\/\/lrclib\.net\//; const WIKIMEDIA = /^https:\/\/((www|query)\.wikidata\.org|[a-z-]+\.wikipedia\.org)\//; const LINES = { @@ -58,18 +59,12 @@ const SERVER = { others: [], }; -async function setUp(page, { server = SERVER, lrclibRows = null, extra = {} } = {}) { +async function setUp(page, { server = SERVER, extra = {} } = {}) { + // Lookups the tab asked the server for (#719). Watched, not answered: a test + // that wants an answer stubs the lookup itself. const lrclib = []; - await page.route(LRCLIB, (route) => { - const url = new URL(route.request().url()); - lrclib.push(url.searchParams.get("artist_name") || `q:${url.searchParams.get("q")}`); - if (!lrclibRows) return route.abort("internetdisconnected"); - return route.fulfill({ - status: 200, - contentType: "application/json", - headers: { "access-control-allow-origin": "*" }, - body: JSON.stringify(lrclibRows(url.searchParams.get("artist_name"))), - }); + page.on("request", (req) => { + if (LYRICS_LOOKUP.test(new URL(req.url()).pathname)) lrclib.push(req.url()); }); await page.route(`**/api/jobs/${JOB_ID}/lyrics`, (route) => (server ? route.fulfill({ status: 200, contentType: "application/json", body: JSON.stringify(server) }) @@ -183,19 +178,19 @@ test.describe("karaoke in Chinese, Japanese and Korean", () => { expect(await ko.evaluate((el) => getComputedStyle(el).wordBreak)).toBe("keep-all"); }); - test("the tab finds a song LRCLIB files under the band's name in its own script", async ({ page }) => { + test("the tab asks the server with the band's names, and shows the song it found", async ({ page }) => { // Tagged "Jay Chou"; LRCLIB has the song only as 周杰倫's, and the artist - // box saved the band with both names. Another singer's 晴天 is never taken. - const rows = (artist) => (artist === "周杰倫" - ? [{ id: 401, trackName: "晴天", artistName: "周杰倫", albumName: "葉惠美", duration: 6, instrumental: false, syncedLyrics: SYNCED, plainLyrics: "" }] - : [{ id: 402, trackName: "晴天", artistName: "张信哲", albumName: "", duration: 6, instrumental: false, syncedLyrics: "[00:00.50]not his", plainLyrics: "" }]); - const { lrclib } = await setUp(page, { + // box saved the band with both names. Searching by the band's other names + // is the server's job now (#719, tests/test_lyrics_lookup.py); the tab + // sends the band it saved and shows what came back. + const rows = [{ id: 401, trackName: "晴天", artistName: "周杰倫", albumName: "葉惠美", duration: 6, instrumental: false, syncedLyrics: SYNCED, plainLyrics: "" }]; + const asked = await stubLyricsLookup(page, { rows, artist: "周杰倫", song: "晴天" }); + await setUp(page, { server: null, - lrclibRows: rows, extra: { artist: { id: "Q238819", name: "周杰倫", englishName: "Jay Chou" } }, }); await expect(page.locator(".lyrics-line")).toHaveCount(4); await expect(page.locator(".lyrics-match-meta")).toHaveText("周杰倫 · 葉惠美"); - expect(lrclib).toEqual(["Jay Chou", "周杰倫"]); + expect(asked.map((a) => a.body.band)).toEqual([{ id: "Q238819", name: "周杰倫", englishName: "Jay Chou" }]); }); }); diff --git a/tests/e2e/lyrics-from-server.spec.mjs b/tests/e2e/lyrics-from-server.spec.mjs index 2c902c9e..58f1cce6 100644 --- a/tests/e2e/lyrics-from-server.spec.mjs +++ b/tests/e2e/lyrics-from-server.spec.mjs @@ -19,9 +19,9 @@ import { stubAudioTags, stubExportEndpoints, stubUpdateCheck, + LYRICS_LOOKUP, } from "./helpers.mjs"; -const LRCLIB = /^https:\/\/lrclib\.net\//; const WIKIMEDIA = /^https:\/\/((www|query)\.wikidata\.org|[a-z-]+\.wikipedia\.org)\//; const version = (id, extra = {}) => ({ @@ -51,10 +51,12 @@ const SERVER = { }; async function setUp(page, { server = SERVER, saved = null } = {}) { + // Lookups the tab asks the server for (#719), answered as LRCLIB being + // out of reach. const lrclib = []; - await page.route(LRCLIB, (route) => { + await page.route(LYRICS_LOOKUP, (route) => { lrclib.push(route.request().url()); - return route.abort("internetdisconnected"); + return route.fulfill({ status: 502, json: { detail: "lyrics service unreachable" } }); }); const served = []; await page.route(`**/api/jobs/${JOB_ID}/lyrics`, (route) => { @@ -208,7 +210,7 @@ test.describe("lyrics from the server", () => { expect(lrclib).toEqual([]); }); - test("with none on the server, the tab looks them up itself as before", async ({ page }) => { + test("with none on the server, the tab asks the server to look them up", async ({ page }) => { const { lrclib, served } = await setUp(page, { server: null }); await expect.poll(() => lrclib.length).toBeGreaterThan(0); expect(served.length).toBeGreaterThan(0); diff --git a/tests/e2e/lyrics.spec.mjs b/tests/e2e/lyrics.spec.mjs index e574a0dd..802a67a5 100644 --- a/tests/e2e/lyrics.spec.mjs +++ b/tests/e2e/lyrics.spec.mjs @@ -3,11 +3,13 @@ // them for the track, and shows them in time with playback, karaoke style. // There is no search box. // -// LRCLIB is answered here, so the tests run offline. The fixture track is six -// seconds long, which is why the synced lines below sit inside it and why the -// first row below counts as the same recording. Ranking, LRC parsing, word -// timing and title cleaning are covered in tests/js/lyrics-lookup.test.mjs; -// this is the page: what is looked up and when, what is kept, what is shown. +// The tab asks the server to look lyrics up (#719), and the server's answer is +// given here (stubLyricsLookup), so the tests run offline. The fixture track +// is six seconds long, which is why the synced lines below sit inside it and +// why the first row below counts as the same recording. Which version the +// server keeps is tested against the server in tests/test_lyrics_lookup.py, +// LRC parsing and word timing in tests/js/lyrics-lookup.test.mjs; this is the +// page: what is asked and when, what is kept, what is shown. import { test, expect } from "@playwright/test"; import { JOB_ID, @@ -19,9 +21,11 @@ import { stubAudioTags, stubExportEndpoints, stubUpdateCheck, + stubLyricsLookup, + LYRICS_LOOKUP, } from "./helpers.mjs"; -const LRCLIB = /^https:\/\/lrclib\.net\//; +const LRCLIB = LYRICS_LOOKUP; const ROWS = [ { @@ -51,20 +55,16 @@ const TAGS = { audioTags: { artist: "Fixture Band", title: "Fixture Song" } }; // The first row as another artist's or another song's, for a search that asks for those. const rowAs = (artistName, trackName) => [{ ...ROWS[0], artistName, trackName }]; -async function stubLrclib(page, { rows = ROWS, offline = false } = {}) { - const asked = []; - await page.route(LRCLIB, async (route) => { - asked.push(route.request().url()); - if (offline) return route.abort("internetdisconnected"); - return route.fulfill({ - status: 200, - contentType: "application/json", - headers: { "access-control-allow-origin": "*" }, - body: JSON.stringify(rows), - }); - }); - return asked; -} +// The tagged artist and song unless a test says otherwise. +const stubLrclib = (page, options = {}) => + stubLyricsLookup(page, { rows: ROWS, artist: "Fixture Band", song: "Fixture Song", ...options }); + +/** What the server kept for the fixture track, as GET .../lyrics answers. */ +const serverKept = (page) => + page.evaluate(async (id) => { + const res = await fetch(`/api/jobs/${id}/lyrics`); + return res.ok ? res.json() : null; + }, JOB_ID); /** Both fixture tracks, the first with `extra` fields (tags, a saved band). */ async function seedWith(page, extra) { @@ -162,15 +162,12 @@ test.describe("lyrics tab", () => { await expect(page.locator(".lyrics-line")).toHaveText(["First line", "Second line", "Third line"]); await expect(page.locator(".lyrics-match-title")).toHaveText("Fixture Song"); - // What went out: the tagged artist and song, to LRCLIB, nothing else. - expect(asked).toHaveLength(1); - const url = new URL(asked[0]); - expect(url.searchParams.get("artist_name")).toBe("Fixture Band"); - expect(url.searchParams.get("track_name")).toBe("Fixture Song"); - expect(asked[0]).not.toContain("E2E"); + // One request, to the server, which looks the tags up itself (#719). + expect(asked).toEqual([{ id: JOB_ID, body: {} }]); - // Kept on this machine, for this track. - await expect.poll(async () => (await stored(page))?.entry?.id ?? null).toBe(101); + // Kept by the server, for this track, the way an import keeps them. + expect((await serverKept(page))?.lrclib_id).toBe(101); + expect(await stored(page)).toBeNull(); // The other version is folded away until asked for, then can be taken. const toggle = page.locator(".lyrics-tools .lyrics-link").first(); @@ -183,15 +180,14 @@ test.describe("lyrics tab", () => { }); test("a saved band is looked up with the song's name taken from the title", async ({ page }) => { - const asked = await stubLrclib(page, { rows: rowAs("Dream Theater", "E2E Fixture Track") }); + const asked = await stubLrclib(page, { rows: rowAs("Dream Theater", "E2E Fixture Track"), artist: "Dream Theater", song: "E2E Fixture Track" }); await seedWith(page, { artist: { id: "Q162586", name: "Dream Theater", englishName: "Dream Theater" } }); await open(page); await openTrack(page); await showLyricsTab(page); await expect(page.locator(".lyrics-line")).toHaveCount(3); - const url = new URL(asked[0]); - expect(url.searchParams.get("artist_name")).toBe("Dream Theater"); - expect(url.searchParams.get("track_name")).toBe("E2E Fixture Track"); + // The band lives in the studio's store, so it goes along with the request. + expect(asked[0].body.band).toEqual({ id: "Q162586", name: "Dream Theater", englishName: "Dream Theater" }); // Kept, so coming back to the tab asks nothing more. await page.locator(".rail-library").click(); @@ -363,9 +359,10 @@ test.describe("lyrics tab", () => { await expect(page.locator("#lyricsStatus")).toHaveText("Lyrics removed for this track."); expect(asked).toHaveLength(1); + // Looking up again finds what the server kept, with nothing more asked. await page.locator(".lyrics-link", { hasText: "Look up again" }).click(); await expect(page.locator(".lyrics-line")).toHaveCount(3); - expect(asked).toHaveLength(2); + expect(asked).toHaveLength(1); }); test("nothing found says so", async ({ page }) => { @@ -422,15 +419,15 @@ test.describe("lyrics tab", () => { }); test("a tagged artist with no tagged title is looked up with the song from the title", async ({ page }) => { - const asked = await stubLrclib(page, { rows: rowAs("Fixture Band", "E2E Fixture Track") }); + // The server takes the song from the title (build_query); the page only + // has to ask, which it does even with no tagged title. + const asked = await stubLrclib(page, { rows: rowAs("Fixture Band", "E2E Fixture Track"), song: "E2E Fixture Track" }); await seedWith(page, { audioTags: { artist: "Fixture Band" } }); await open(page); await openTrack(page); await showLyricsTab(page); await expect(page.locator(".lyrics-line")).toHaveCount(3); - const url = new URL(asked[0]); - expect(url.searchParams.get("artist_name")).toBe("Fixture Band"); - expect(url.searchParams.get("track_name")).toBe("E2E Fixture Track"); + expect(asked).toHaveLength(1); }); test("another artist's song of the same name is not this track's lyrics", async ({ page }) => { diff --git a/tests/e2e/search.spec.mjs b/tests/e2e/search.spec.mjs index 7c01c51d..6ea0f0ff 100644 --- a/tests/e2e/search.spec.mjs +++ b/tests/e2e/search.spec.mjs @@ -150,7 +150,7 @@ test.describe("topbar search", () => { await expect(page.locator("#url")).toHaveValue(/youtube\.com\/watch\?v=/); await expect(panel(page)).toBeHidden(); - // Extraction is minutes of work. It waits for Split stems. + // Extraction is minutes of work. It waits for Extract stems. expect(jobPosts).toHaveLength(0); }); diff --git a/tests/e2e/slow-speed.spec.mjs b/tests/e2e/slow-speed.spec.mjs new file mode 100644 index 00000000..c56f8d0b --- /dev/null +++ b/tests/e2e/slow-speed.spec.mjs @@ -0,0 +1,50 @@ +// The slow speed is the player's to set (#701). +// +// Speed was a fixed 0.75x or 1x. Some fills want 0.75x and some only a nudge, +// so the slow button can be moved a hundredth at a time, by the wheel or the +// up and down arrow keys, between 0.50x and 0.99x, and it is remembered. + +import { test, expect } from "@playwright/test"; +import { openStudio } from "./helpers.mjs"; + +const slow = (page) => page.locator("#t-speed-075"); +const rate = (page) => page.evaluate(async () => (await import("/js/state.js")).playbackSpeed); + +test("scrolling over the slow button moves it a hundredth at a time and plays at it", async ({ page }) => { + await openStudio(page); + await expect(slow(page)).toHaveText("0.75x"); + await slow(page).hover(); + await page.mouse.wheel(0, -100); + await page.mouse.wheel(0, -100); + await expect(slow(page)).toHaveText("0.77x"); + await expect(slow(page)).toHaveAttribute("aria-checked", "true"); + await expect.poll(() => rate(page)).toBe(0.77); + + // 1x is still one press away, and the slow speed is kept for next time. + await page.locator("#t-speed-1").click(); + await expect.poll(() => rate(page)).toBe(1); + await slow(page).click(); + await expect.poll(() => rate(page)).toBe(0.77); +}); + +test("the arrow keys move it too, and it stops at 0.50x and 0.99x", async ({ page }) => { + await openStudio(page); + await slow(page).focus(); + for (let i = 0; i < 40; i++) await page.keyboard.press("ArrowDown"); + await expect(slow(page)).toHaveText("0.50x"); + for (let i = 0; i < 60; i++) await page.keyboard.press("ArrowUp"); + await expect(slow(page)).toHaveText("0.99x"); + await expect.poll(() => rate(page)).toBe(0.99); +}); + +test("the slow speed survives a reload", async ({ page }) => { + await openStudio(page); + await slow(page).hover(); + await page.mouse.wheel(0, 100); + await expect(slow(page)).toHaveText("0.74x"); + await page.waitForTimeout(400); + await page.reload({ waitUntil: "domcontentloaded" }); + await expect(slow(page)).toHaveText("0.74x"); + // A track opens at normal speed, as it always has. + await expect(page.locator("#t-speed-1")).toHaveAttribute("aria-checked", "true"); +}); diff --git a/tests/js/lyrics-lookup.test.mjs b/tests/js/lyrics-lookup.test.mjs index 31173f7c..cb7eb74f 100644 --- a/tests/js/lyrics-lookup.test.mjs +++ b/tests/js/lyrics-lookup.test.mjs @@ -1,30 +1,20 @@ -// The Lyrics tab's lookup (#699): how LRC text becomes timed lines, which line -// is being sung at a given moment, which of LRCLIB's many versions of a song -// is taken as the one, and what is sent to search for it. -// -// No network. searchLyrics takes its fetch as an argument. +// The Lyrics tab's reading of lyrics (#699): how LRC text becomes timed lines, +// which line is being sung at a given moment, and what the server's answer +// becomes. Which of LRCLIB's versions is the track's is the server's to decide +// (#719), and is tested in tests/test_lyrics_lookup.py. // // Run: node tests/js/lyrics-lookup.test.mjs import { parseLrc, currentLineIndex, - rankMatches, - searchUrl, - searchLyrics, wordTimings, voicedPhrases, syllables, songFromTitle, fromServerLyrics, - strippedCopy, - belongsTo, - sameArtist, sameSong, - scriptNames, - otherNames, fold as foldName, - rankVersions, clampOffset, shiftLines, firstSungIndex, @@ -208,66 +198,6 @@ check("between lines, the one before", currentLineIndex(lines, 25) === 1); check("after the last", currentLineIndex(lines, 99) === 2); check("no lines", currentLineIndex([], 5) === -1); -// ── rankMatches ── -const row = (id, duration, kind) => ({ - id, - trackName: `T${id}`, - artistName: "A", - albumName: "", - duration, - instrumental: kind === "instrumental", - syncedLyrics: kind === "synced" ? "[00:01.00]x" : null, - plainLyrics: kind === "synced" || kind === "plain" ? "x" : null, -}); -const ranked = rankMatches( - [row(1, 769, "synced"), row(2, 571, "plain"), row(3, 573, "synced"), row(4, 572, "none"), row(5, 590, "synced")], - 572, -); -check( - "closest length first, synced preferred within a few seconds, empty rows dropped", - same(ranked.map((m) => m.id), [3, 2, 5, 1]), - JSON.stringify(ranked.map((m) => m.id)), -); -check("an instrumental row is kept", rankMatches([row(9, 100, "instrumental")], 100).length === 1); -check( - "with no length known, LRCLIB's order, synced first", - same(rankMatches([row(1, 10, "plain"), row(2, 500, "synced")], 0).map((m) => m.id), [2, 1]), -); -check("not an array gives nothing", same(rankMatches(null), []) && same(rankMatches({ error: 1 }), [])); - -// ── searchUrl ── -const withArtist = new URL(searchUrl({ artist: " Dream Theater ", song: " Metropolis " })); -check("LRCLIB's search", withArtist.origin + withArtist.pathname === "https://lrclib.net/api/search"); -check( - "artist and song as their own fields, trimmed", - withArtist.searchParams.get("artist_name") === "Dream Theater" && withArtist.searchParams.get("track_name") === "Metropolis", -); -const songOnly = new URL(searchUrl({ song: "Metropolis" })); -check( - "no artist: the free-text search, not an empty artist filter", - songOnly.searchParams.get("q") === "Metropolis" && !songOnly.searchParams.has("artist_name"), -); - -// ── searchLyrics ── -const asked = []; -const found = await searchLyrics( - { artist: "Dream Theater", song: "Metropolis", duration: 572 }, - { fetchJson: async (url) => { asked.push(url); return [row(1, 769, "synced"), row(3, 573, "synced")]; } }, -); -check("ranked for the track's length", same(found.map((m) => m.id), [3, 1])); -check("one request, to LRCLIB only", asked.length === 1 && asked[0].startsWith("https://lrclib.net/")); -check( - "no song asks nothing", - same(await searchLyrics({ artist: "X", song: " " }, { fetchJson: () => { throw new Error("asked"); } }), []), -); -let rejected = false; -try { - await searchLyrics({ song: "x" }, { fetchJson: async () => { throw new Error("offline"); } }); -} catch { - rejected = true; -} -check("no connection rejects, rather than reading as nothing found", rejected); - // ── fromServerLyrics: lyrics.json from GET /api/jobs/{id}/lyrics ── const version = (id, source = "lrclib", extra = {}) => ({ v: 1, @@ -319,70 +249,6 @@ check( ); // ── letters outside ASCII ── -// LRCLIB holds some songs as copies that lost those letters on the way in -// (Kayah's "Nie ma, nie ma ciebie": eleven copies read "Niewinnoci biaym -// niegiem", one "Niewinnością białym śniegiem"). The anthem stands in for a -// song, being in the public domain. The same cases as tests/test_lyrics_lookup.py. -const ANTHEM = [ - "[00:01.00]Jeszcze Polska nie zginęła,", - "[00:04.00]Kiedy my żyjemy.", - "[00:07.00]Co nam obca przemoc wzięła,", - "[00:10.00]Szablą odbierzemy.", - "[00:13.00]Marsz, marsz, Dąbrowski,", - "[00:16.00]Z ziemi włoskiej do Polski.", - "[00:19.00]Za twoim przewodem", - "[00:22.00]Złączym się z narodem.", -].join("\n"); -const drop = (s) => s.replace(/[^\p{ASCII}]/gu, ""); -const fold = (s) => drop(s.normalize("NFD")); -const asLrclibDropsThem = (s) => drop(s.replaceAll("ę", "e")); -const textRow = (id, duration, text, synced = true) => ({ - id, - trackName: `T${id}`, - artistName: "A", - albumName: "", - duration, - instrumental: false, - syncedLyrics: synced ? text : null, - plainLyrics: text.replace(/\[[^\]\n]*\]/g, ""), -}); -const lyricsOf = (text) => ({ synced: text, plain: "" }); - -for (const [name, strip] of [["dropped", drop], ["folded", fold], ["as LRCLIB drops them", asLrclibDropsThem]]) { - check(`a copy whose Polish letters were ${name} is told from its source`, strippedCopy(lyricsOf(strip(ANTHEM)), lyricsOf(ANTHEM))); -} -check("the intact copy is not a stripped one", !strippedCopy(lyricsOf(ANTHEM), lyricsOf(asLrclibDropsThem(ANTHEM)))); -check("a copy is not a stripped copy of itself", !strippedCopy(lyricsOf(ANTHEM), lyricsOf(ANTHEM))); -for (const [intact, other] of [ - ["[00:01.00]Группа крови на рукаве, мой порядковый номер на рукаве", "[00:01.00]Gruppa krovi na rukave, moy poryadkovyy nomer na rukave"], - ["[00:01.00]夢ならばどれほどよかったでしょう 未だにあなたのことを夢にみる", "[00:01.00]Yume naraba dore hodo yokatta deshou imada ni anata no koto wo yume ni miru"], - ["[00:01.00]A café, a naïve smile, and the rest in plain English words", "[00:01.00]A cafe, a naive smile, and the rest in plain English words"], - [ANTHEM, "[00:01.00]Jeszcze nic nie jest stracone, moja mila, gdy jestem z toba"], -]) { - check( - `not a stripped copy: ${other.slice(10, 30)}`, - !strippedCopy(lyricsOf(other), lyricsOf(intact)) && !strippedCopy(lyricsOf(intact), lyricsOf(other)), - ); -} -{ - const rows = [ - textRow(5470091, 230, asLrclibDropsThem(ANTHEM)), - textRow(28700266, 230, asLrclibDropsThem(ANTHEM)), - textRow(10910419, 229.93, ANTHEM), - textRow(4291789, 230, ANTHEM, false), - ]; - const want = [10910419, 5470091, 28700266, 4291789]; - check("the intact copy ranks before stripped ones LRCLIB listed first", same(rankMatches(rows, 230).map((m) => m.id), want)); - check("and with the length unknown", same(rankMatches(rows).map((m) => m.id), want)); - const stripped = textRow(1, 230, asLrclibDropsThem(ANTHEM)); - check( - "synced still beats plain, and length beats both", - same(rankMatches([stripped, textRow(2, 230, ANTHEM, false)], 230).map((m) => m.id), [1, 2]) - && same(rankMatches([stripped, textRow(3, 250, ANTHEM)], 230).map((m) => m.id), [1, 3]), - ); -} - -// Polish through parsing and the wipe: nothing dropped, nothing split. { const polish = parseLrc("[00:31.72]<00:31.72>Śpiewałem <00:32.40>głośno <00:32.90>pod <00:33.10>prysznicem\n[00:33.97]Ten mój małomiasteczkowy hit"); check("Polish lines keep every letter", polish[0].text === "Śpiewałem głośno pod prysznicem" && polish[1].text === "Ten mój małomiasteczkowy hit"); @@ -403,52 +269,17 @@ for (const [intact, other] of [ check("Cyrillic words stay whole", same(cyrillic.map((w) => w.text), ["Группа ", "крови ", "на ", "рукаве"])); } -// Whose song a version is: only this song by this artist is ever kept. +// Whose song kept lyrics are: the check on what the tab kept before the +// server held versions to the track's song (#719). The rest of the rule is +// the server's alone (tests/test_lyrics_lookup.py). { - const ask = { artist: "Keala Settle & The Greatest Showman Ensemble", song: "This Is Me" }; - const row = (artist, track) => ({ artist, track }); - check("the same artist and song belong", belongsTo(row("Keala Settle & The Greatest Showman Ensemble", "This Is Me"), ask)); - check("one credited artist belongs", belongsTo(row("Keala Settle", "This Is Me"), ask)); - check("the cast filed as a run of the name belongs", belongsTo(row("The Greatest Showman Ensemble", "This Is Me"), ask)); - check("a song tail belongs", belongsTo(row("Keala Settle", "This Is Me - From The Greatest Showman"), ask)); check("brackets and featuring are ignored", sameSong("This Is Me (feat. Someone) [Live]", "This Is Me")); - check("another artist's song of that name does not", !belongsTo(row("Kesha", "This Is Me"), ask)); - check("the artist's other song does not", !belongsTo(row("Keala Settle", "This Is Not Me"), ask)); + check("a song tail belongs", sameSong("This Is Me - From The Greatest Showman", "This Is Me")); + check("another song is not this one", !sameSong("This Is Not Me", "This Is Me")); check("two different tails are two songs", !sameSong("Part I - Dawn", "Part I - Dusk")); - check("no artist known, nothing belongs", !belongsTo(row("Keala Settle", "This Is Me"), { artist: "", song: "This Is Me" })); - check("a show's name counts when given", belongsTo(row("Wicked", "Popular"), { artist: "Kristin Chenoweth", song: "Popular", names: ["Wicked"] })); - check("a show's name does not count unless given", !belongsTo(row("Wicked", "Popular"), { artist: "Kristin Chenoweth", song: "Popular" })); - check("case, accents and a leading The do not matter", sameArtist("the beatles", ["The Beatles"]) && sameArtist("Beyonce", ["Beyoncé"]) && sameArtist("Beatles", ["The Beatles"])); - check("Polish names compare letter for letter", sameArtist("Dawid Podsiadło", ["Dawid Podsiadło"]) && !sameArtist("Dawid Podsiadło", ["Dawid Kwiatkowski"])); - check("a two-letter credit names nobody", !sameArtist("DJ", ["DJ & Someone Else"])); - check("one shared word is not a shared name", !sameArtist("Pink", ["Pink Floyd"])); } -// Chinese, Japanese and Korean names: the cases in tests/test_lyrics_names.py. { - const artists = [ - ["周杰伦", ["周杰倫"], true], - ["邓丽君", ["鄧麗君"], true], - ["ＹＯＡＳＯＢＩ", ["YOASOBI"], true], - ["ｷﾝｸﾞﾇｰ", ["キングヌー"], true], - ["Dawid Podsiadlo", ["Dawid Podsiadło"], true], - ["鄧麗君 (Teresa Teng)", ["Teresa Teng"], true], - ["五月天 (Mayday)", ["五月天"], true], - ["IU", ["IU (아이유)"], true], - ["周杰倫", ["周杰倫 Jay Chou"], true], - ["Jay Chou", ["周杰倫 Jay Chou"], true], - ["周杰倫 & 費玉清", ["周杰倫"], true], - ["Jay Chou", ["周杰倫"], false], - ["아이유", ["IU"], false], - ["张信哲", ["周杰倫"], false], - ["五月天 阿信", ["五月天"], false], - ["告五人", ["五月天"], false], - ["Official", ["Official髭男dism"], false], - ["林", ["林 & 周杰倫"], false], - ]; - for (const [found, names, want] of artists) { - check(`sameArtist(${found}, ${names}) is ${want}`, sameArtist(found, names) === want); - } const songs = [ ["红豆", "紅豆", true], ["晴天 (Sunny Day)", "晴天", true], @@ -465,20 +296,7 @@ for (const [intact, other] of [ for (const [found, song, want] of songs) { check(`sameSong(${found}, ${song}) is ${want}`, sameSong(found, song) === want); } - check("a name in two scripts gives both", same(scriptNames("周杰倫 Jay Chou"), ["周杰伦", "Jay Chou"]) && same(scriptNames("IU(아이유)"), ["IU", "아이유"])); - check("a name mixing scripts in one word is one name", same(scriptNames("Official髭男dism"), []) && same(scriptNames("五月天 阿信"), [])); check("folding: traditional, width, plain Latin", foldName("鄧麗君 ＩＵ Podsiadło") === "邓丽君 IU Podsiadlo"); - check("the band's names count as the artist's", belongsTo({ artist: "周杰倫", track: "晴天" }, { artist: "Jay Chou", song: "晴天", names: ["周杰倫", "Jay Chou"] })); - check("and never make another artist's song the track's", !belongsTo({ artist: "张信哲", track: "晴天" }, { artist: "Jay Chou", song: "晴天", names: ["周杰倫"] })); - check( - "other names to search by: a name's own scripts, then the band's, each once", - same(otherNames("周杰倫 Jay Chou", ["周杰倫", "Jay Chou", "周杰倫 Jay Chou"]), ["周杰伦", "Jay Chou", "周杰倫"]), - JSON.stringify(otherNames("周杰倫 Jay Chou", ["周杰倫", "Jay Chou"])), - ); - check("the artist's own name is not searched again", same(otherNames("IU", ["IU", "아이유"]), ["아이유"])); - const a = rankMatches([row(1, 300, "synced"), row(2, 269, "synced")], 269); - const b = rankMatches([row(3, 270, "synced"), row(2, 269, "synced")], 269); - check("versions from two searches rank together, each once", same(rankVersions([...a, ...b], 269).map((m) => m.id), [2, 3, 1])); } // Karaoke in Chinese, Japanese and Korean. diff --git a/tests/js/name-parity.test.mjs b/tests/js/name-parity.test.mjs index 854a0a05..47a9694e 100644 --- a/tests/js/name-parity.test.mjs +++ b/tests/js/name-parity.test.mjs @@ -1,6 +1,8 @@ -// JS half of the name parity gate: the page's lyrics matching -// (static/js/lyricsLookup.js, zhVariants.js) held to what the server's -// (app/pipeline/lyrics_lookup.py, name_aliases.py, zh_variants.py) says. +// JS half of the name parity gate: what is left of the page's name +// handling (static/js/lyricsLookup.js fold and sameSong, zhVariants.js) held +// to what the server's (app/pipeline/lyrics_lookup.py, name_aliases.py, +// zh_variants.py) says. The page no longer decides whose song a version is +// (#719); sameSong only checks lyrics it kept before that rule existed. // // tests/fixtures/name_parity.json is generated from Python and asserted by // tests/test_name_parity.py; this file asserts the page gives the same @@ -14,7 +16,7 @@ // Run: node tests/js/name-parity.test.mjs import { readFileSync } from 'node:fs'; -import { fold, scriptNames, sameArtist, sameSong } from '../../static/js/lyricsLookup.js'; +import { fold, sameSong } from '../../static/js/lyricsLookup.js'; import { PAIRS, toSimplified } from '../../static/js/zhVariants.js'; const fixture = JSON.parse(readFileSync(new URL('../fixtures/name_parity.json', import.meta.url), 'utf8')); @@ -31,22 +33,12 @@ function check(name, condition, detail = '') { } } -const same = (a, b) => JSON.stringify(a) === JSON.stringify(b); - -check('the fixture is not empty', fixture.fold.length >= 10 && fixture.sameArtist.length >= 20 && fixture.sameSong.length >= 10); +check('the fixture is not empty', fixture.fold.length >= 10 && fixture.sameSong.length >= 10); for (const { in: text, out } of fixture.fold) { const got = fold(text); check(`fold(${JSON.stringify(text)})`, got === out, `page ${JSON.stringify(got)}, server ${JSON.stringify(out)}`); } -for (const { in: name, out } of fixture.scriptNames) { - const got = scriptNames(name); - check(`scriptNames(${JSON.stringify(name)})`, same(got, out), `page ${JSON.stringify(got)}, server ${JSON.stringify(out)}`); -} -for (const { found, names, same: want } of fixture.sameArtist) { - const got = sameArtist(found, names); - check(`sameArtist(${JSON.stringify(found)}, ${JSON.stringify(names)})`, got === want, `page ${got}, server ${want}`); -} for (const { found, song, same: want } of fixture.sameSong) { const got = sameSong(found, song); check(`sameSong(${JSON.stringify(found)}, ${JSON.stringify(song)})`, got === want, `page ${got}, server ${want}`); diff --git a/tests/js/source-arming.test.mjs b/tests/js/source-arming.test.mjs index 42ac1c24..81497239 100644 --- a/tests/js/source-arming.test.mjs +++ b/tests/js/source-arming.test.mjs @@ -1,4 +1,4 @@ -// Which track sources the Split stems button may be pointed at (#635). +// Which track sources the Extract stems button may be pointed at (#635). // // The composer is an input that button submits, not a caption for the open // track. An uploaded file's source is the synthetic "local:my song.mp3", and diff --git a/tests/test_csp.py b/tests/test_csp.py index 9b8e1a11..6a0ecb20 100644 --- a/tests/test_csp.py +++ b/tests/test_csp.py @@ -22,18 +22,19 @@ def test_connect_src_permits_data_and_blob(): assert "blob:" in connect -def test_connect_src_allows_the_artist_and_lyrics_hosts_and_nothing_wider(): - # The artist box and the Lyrics tab (#699) read Wikidata, Wikipedia and - # LRCLIB straight from the page. Those hosts by name, never a bare https: - # that would let an injected script send anything anywhere. +def test_connect_src_allows_the_artist_hosts_and_nothing_wider(): + # The artist box (#699) reads Wikidata and Wikipedia straight from the + # page. Those hosts by name, never a bare https: that would let an + # injected script send anything anywhere. LRCLIB is not among them: the + # Lyrics tab asks the server to look lyrics up (#719). sources = _csp_directive("connect-src").split()[1:] for host in ( "https://www.wikidata.org", "https://query.wikidata.org", "https://*.wikipedia.org", - "https://lrclib.net", ): assert host in sources + assert "https://lrclib.net" not in sources assert "https:" not in sources assert "*" not in sources diff --git a/tests/test_jobs_favorite.py b/tests/test_jobs_favorite.py new file mode 100644 index 00000000..f388cc50 --- /dev/null +++ b/tests/test_jobs_favorite.py @@ -0,0 +1,83 @@ +"""Favourites live on the server, so the phone can set them and filter by them (#734). + +They used to live only in the desktop page's catalog store. The phone builds its +library from GET /api/jobs and had no way to reach that store, so its Favorites +chip listed every track and there was no heart to press. + +`favorite` starts as None, meaning no client has said either way. The desktop +uses that to hand up favourites it set before the field existed, without +undoing one taken back out on the phone. +""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +from app.core.models import Job +from app.core.registry import _jobs, register +from app.main import app + + +@pytest.fixture(autouse=True) +def _isolate_registry(): + _jobs.clear() + yield + _jobs.clear() + + +@pytest.fixture +def client() -> TestClient: + return TestClient(app) + + +def _done(job_id: str) -> Job: + job = Job(id=job_id, status="done", title="Song") + register(job) + return job + + +def test_a_new_job_has_said_nothing_about_favourites(client: TestClient) -> None: + _done("aaaaaaaaaaaa") + assert client.get("/api/jobs/aaaaaaaaaaaa").json()["favorite"] is None + + +def test_favourite_and_back_out(client: TestClient) -> None: + _done("aaaaaaaaaaaa") + resp = client.put("/api/jobs/aaaaaaaaaaaa/favorite", json={"favorite": True}) + assert resp.status_code == 200 + assert resp.json() == {"job_id": "aaaaaaaaaaaa", "favorite": True} + assert client.get("/api/jobs").json()[0]["favorite"] is True + + resp = client.put("/api/jobs/aaaaaaaaaaaa/favorite", json={"favorite": False}) + assert resp.json()["favorite"] is False + # False, not None: taken back out is an answer the desktop must respect. + assert client.get("/api/jobs/aaaaaaaaaaaa").json()["favorite"] is False + + +def test_the_flag_survives_a_restart() -> None: + job = _done("aaaaaaaaaaaa") + job.favorite = True + assert Job.from_record(job.to_record()).favorite is True + + +@pytest.mark.parametrize("body", [{}, {"favorite": "false"}, {"favorite": 1}, {"favorite": None}]) +def test_anything_but_a_bool_is_refused(client: TestClient, body: dict) -> None: + _done("aaaaaaaaaaaa") + resp = client.put("/api/jobs/aaaaaaaaaaaa/favorite", json=body) + assert resp.status_code == 422 + assert client.get("/api/jobs/aaaaaaaaaaaa").json()["favorite"] is None + + +def test_unknown_job_is_404(client: TestClient) -> None: + assert client.put("/api/jobs/999999999999/favorite", json={"favorite": True}).status_code == 404 + + +@pytest.mark.parametrize( + "job_id", ["../../etc/passwd", "..%2F..%2Fetc", "not-a-job-id", "AAAAAAAAAAAA"] +) +def test_crafted_ids_are_refused_not_500(client: TestClient, job_id: str) -> None: + """Some are collapsed before routing and nothing answers PUT there (405); + the rest arrive as a job id and are refused (404). Never a 500.""" + resp = client.put(f"/api/jobs/{job_id}/favorite", json={"favorite": True}) + assert resp.status_code in (404, 405) diff --git a/tests/test_lyrics_api.py b/tests/test_lyrics_api.py index 3aff16c2..d45056aa 100644 --- a/tests/test_lyrics_api.py +++ b/tests/test_lyrics_api.py @@ -648,3 +648,86 @@ def test_other_versions_of_another_song_are_not_offered(client): _lyrics_file(job).write_text(json.dumps({**LYRICS, "others": others}), encoding="utf-8") got = client.get(f"/api/jobs/{job.id}/lyrics").json() assert [o["lrclib_id"] for o in got["others"]] == [8] + + +# ── POST .../lyrics/lookup: the tab asks the server, not LRCLIB (#719) ── + + +def test_lookup_keeps_lyrics_the_way_the_import_does(client): + job = _done_job(audio_tags=TAGS) + lrclib = Lrclib(ROWS) + with patch.object(ll, "_fetch_json", lrclib): + r = client.post(f"/api/jobs/{job.id}/lyrics/lookup", json={}) + assert r.status_code == 200 + assert r.json()["synced"] == "[00:01.00]The smile of dawn" + assert _lyrics_file(job).is_file() + assert job.has_lyrics is True + # And GET answers the same from now on, with no second request. + assert client.get(f"/api/jobs/{job.id}/lyrics").json()["synced"] == r.json()["synced"] + + +def test_lookup_uses_the_band_the_studio_saved(client): + """No tags: only the band chosen in the artist box, which the studio + keeps in its own store, says whose song this is.""" + job = _done_job() + lrclib = Lrclib(ROWS) + with patch.object(ll, "_fetch_json", lrclib): + r = client.post(f"/api/jobs/{job.id}/lyrics/lookup", json={"band": DT}) + assert r.status_code == 200 + assert lrclib.asked + + +def test_lookup_with_nothing_to_go_on_asks_nobody(client): + job = _done_job() + job.title = "" + lrclib = Lrclib(ROWS) + with patch.object(ll, "_fetch_json", lrclib): + r = client.post(f"/api/jobs/{job.id}/lyrics/lookup", json={}) + assert r.status_code == 404 + assert r.json()["nothing_known"] is True + assert lrclib.asked == [] + + +def test_lookup_offers_other_artists_nothing(client): + """Another artist's song of the same name is not this track's lyrics: + the server's rule, now the only one.""" + job = _done_job(audio_tags=TAGS) + other = [{**ROWS[0], "artistName": "Someone Else"}] + with patch.object(ll, "_fetch_json", Lrclib(other)): + r = client.post(f"/api/jobs/{job.id}/lyrics/lookup", json={}) + assert r.status_code == 404 + assert r.json().get("others", []) == [] + assert not _lyrics_file(job).is_file() + + +def test_lookup_offline_is_502_so_the_tab_can_retry(client): + job = _done_job(audio_tags=TAGS) + # conftest's stand-in answers every request as offline. + r = client.post(f"/api/jobs/{job.id}/lyrics/lookup", json={}) + assert r.status_code == 502 + assert r.json() == {"detail": "lyrics service unreachable"} + + +def test_lookup_answers_kept_lyrics_without_asking(client): + job = _done_job(has_lyrics=True, audio_tags=TAGS) + _lyrics_file(job).write_text(json.dumps(LYRICS), encoding="utf-8") + lrclib = Lrclib(ROWS) + with patch.object(ll, "_fetch_json", lrclib): + r = client.post(f"/api/jobs/{job.id}/lyrics/lookup", json={}) + assert r.status_code == 200 + assert lrclib.asked == [] + + +@pytest.mark.parametrize("band", [{"id": "../etc", "name": "x"}, {"id": "Q1", "name": "x" * 301}]) +def test_lookup_refuses_a_malformed_band(client, band): + job = _done_job() + r = client.post(f"/api/jobs/{job.id}/lyrics/lookup", json={"band": band}) + assert r.status_code == 422 + + +@pytest.mark.parametrize( + "job_id", ["../../etc/passwd", "..%2F..%2Fetc", "not-a-job", "bbbbbbbbbbbb"] +) +def test_lookup_crafted_or_unknown_ids_are_refused_not_500(client, job_id): + r = client.post(f"/api/jobs/{job_id}/lyrics/lookup", json={}) + assert r.status_code in (404, 405)