diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 114aa0b..239163e 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -13,7 +13,7 @@ desktop application shell ├── retune-spotify OAuth, Web API, normalization ├── retune-audio local-file scan, tags, decoding ├── playback controller + Spotify/file backends - ├── lastfm_import account-bound snapshot, matching, review, and apply boundary + ├── lastfm_import account-bound snapshot, lazy matching, review, and apply boundary └── store app-data persistence ``` @@ -39,8 +39,9 @@ shell owns orchestration and persistence. Domain crates do not depend on Tauri. - The playback controller owns the canonical queue, active order, generation, and user-facing playback state. - React owns view selection, navigation, dialog state, and transient gestures. -- `lastfm_import` owns the resumable Last.fm snapshot, compact source variants, - Spotify matching results, review decisions, and account-bound application. +- `lastfm_import` owns the resumable Last.fm snapshot, raw-page cache, compact + source variants, lazy Spotify matching results, review decisions, and + account-bound application. Its parsing and review helpers do not add network or filesystem concerns to `retune-core`. @@ -69,31 +70,70 @@ updates state, advances the queue, and records threshold-based play counts. ### Last.fm import The Preferences action opens a second `lastfm-importer` WebviewWindow at -1320×840. The importer captures one fixed Last.fm `to` timestamp, fetches -`user.getRecentTracks` sequentially at 200 rows per page, skips now-playing and -undated rows, and atomically checkpoints compact aggregate variants plus the -next page. Its session defaults independently control content and historical -play counts (both on by default, with whole-album content mode off); at least -one remains selected. Matching then runs sequentially through the shared -Spotify client; album candidates are limited to ten and classified from real -track-set overlap without monopolizing the membership gate. +1320×840. An account-bound `lastfmScrobblingProfile` records the first +successful connection/enable timestamp for each Last.fm username; toggling +preserves the same username's timestamp and a different username replaces it. +The importer captures that fixed `historyTo`, probes metadata once, and fetches +`user.getRecentTracks` at its documented 200-row limit from the oldest page +toward page 1. It skips now-playing and undated rows, writes parsed raw pages +under a snapshot-specific machine cache, discards rows at or after `historyTo` +before caching or counting, acknowledges each page in a manifest only after +the atomic page write, and enforces 100 MiB session/cache ceilings. The exact +Last.fm username is recorded in both manifest and page metadata. No +aggregation or Spotify request occurs during source work. Once every manifest +page is present, raw-page reads, sorting, and aggregation run off the async +runtime; the importer then atomically enters review, or Done when no rows remain, +before best-effort cache cleanup. + +The source runner persists retry state after Last.fm's internal capped retry is +exhausted, waits at the capped delay, and retries the same probe/page in +process while the app remains running; a failed request never advances its +cursor. An acknowledged missing, corrupt, oversized, or metadata-mismatched +page quarantines the snapshot and starts a fresh V2 session. An unacknowledged +page file is an ignorable orphan and can be overwritten on retry. V1 caches are +quarantined because the fixed cutoff changes page boundaries. Relaunching with +a saved Downloading or Aggregating session resumes it once after hydration; +an empty state never creates a session implicitly. + +Opening a visible review batch lazily searches through the shared Spotify +client/request gate, serializes duplicate batch requests, binds the Spotify +account on the first match, and caches the results. Review batches are +persisted as stable `ImportBatch` pages capped at 100 source rows; large +artist/album groups, including singles, are split without changing the +artist-level cascade. Commands validate the page and source IDs, and fuzzy +disclosures stay inside the visible batch while count modes remain target-wide. +Revisiting a cached batch does not call Spotify and there is no adjacent +prefetch. Accept All is the explicit bulk exception: it sequentially prepares +every remaining batch, shows global unique album/track URI counts, then applies +only after confirmation. Fuzzy count strategies are persisted once per Spotify track target for the -session, and the review page discloses every source row in the session that -resolves to that target. “Show Spotify search terms” is likewise one persisted -session preference, restored when the importer resumes rather than copied into -each page’s options. +session, while fuzzy disclosures are bounded to the visible persisted batch. +The target-wide count decision still includes completed source rows from other +batches. “Show Spotify search terms” is likewise one persisted session +preference, restored when the importer resumes rather than copied into each +page’s options. Whole-album acceptance sends one album URI to Spotify and updates `SavedAlbumRecord`; selected-track acceptance sends only track URIs and updates `saved_tracks`. Upstream membership completes before the atomic local history and metadata mutation, and a durable decision is marked done only after that -mutation succeeds. The session is bound to both the Last.fm username and -Spotify `/me` account ID; a mismatch suspends it. +mutation succeeds. The source session is bound to Last.fm first; Spotify +`/me` is nullable until the first lazy match. A later Spotify mismatch +suspends Spotify-derived work without invalidating the source snapshot. The importer serializes each session mutation through durable replacement before updating memory, and matching rechecks the expected account and phase at every -durable checkpoint and before entering review. Suspended state exposes no prior -account identity or queue. +durable checkpoint and before entering review. Cached Spotify-derived pages +trust only an exact cached library identity; otherwise the current `/me` account +is resolved. Post-search ownership validation and match persistence hold the +shared Spotify membership gate together. Suspended state exposes no prior +account identity or queue. Source-phase Downloading/Aggregating state remains +Spotify-free; bound Review/Done reads and bound Suspended resume validate exact +Spotify ownership under the shared gate, while an unbound source suspension +requires only Last.fm. After Last.fm hydration, the Tauri shell—not React— +claims and starts one persisted Downloading/Aggregating runner using its stored +username and cutoff. React only observes progress and offers explicit +first-start/manual-resume actions. ## Cross-cutting rules diff --git a/apps/desktop/src-tauri/src/lastfm.rs b/apps/desktop/src-tauri/src/lastfm.rs index 3ff337d..30360f5 100644 --- a/apps/desktop/src-tauri/src/lastfm.rs +++ b/apps/desktop/src-tauri/src/lastfm.rs @@ -1,6 +1,7 @@ use std::{ collections::VecDeque, fs::{self, OpenOptions}, + future::Future, io::Write, path::{Path, PathBuf}, sync::Arc, @@ -357,6 +358,18 @@ pub(crate) struct ImportFetchError { pub account_mismatch: bool, } +fn import_recent_tracks_params(username: &str, page: u32, to: u64) -> Vec<(String, String)> { + vec![ + ("user".into(), username.into()), + ("page".into(), page.to_string()), + ( + "limit".into(), + crate::lastfm_import::LASTFM_PAGE_LIMIT.to_string(), + ), + ("to".into(), to.to_string()), + ] +} + impl Failure { fn code(self) -> Option { match self { @@ -676,6 +689,28 @@ impl Service { } } + pub(crate) async fn with_import_owner( + &self, + username: &str, + operation: F, + ) -> Result, String> + where + F: FnOnce() -> Fut + Send, + Fut: Future> + Send, + T: Send, + { + let _runtime = self.runtime.lock().await; + if _runtime + .session + .as_ref() + .map(|session| session.username.as_str()) + != Some(username) + { + return Ok(None); + } + Ok(Some(operation().await?)) + } + pub(crate) async fn set_enabled(self: &Arc, enabled: bool) { let should_flush = { let mut runtime = self.runtime.lock().await; @@ -907,15 +942,7 @@ impl Service { account_mismatch: true, }); } - let params = vec![ - ("user".into(), username.into()), - ("page".into(), page.to_string()), - ( - "limit".into(), - crate::lastfm_import::LASTFM_PAGE_LIMIT.to_string(), - ), - ("to".into(), to.to_string()), - ]; + let params = import_recent_tracks_params(username, page, to); for attempt in 0..=RETRY_DELAYS.len() { match self .post("user.getRecentTracks", params.clone(), None) @@ -1547,6 +1574,10 @@ fn retry_delay(attempt: usize) -> Duration { RETRY_DELAYS[attempt.min(RETRY_DELAYS.len() - 1)] } +pub(crate) fn import_retry_delay(attempt: usize) -> Duration { + retry_delay(attempt) +} + pub(crate) fn scrobble_threshold_ms(duration_secs: u64) -> Option { (duration_secs > 30).then(|| (duration_secs.saturating_mul(500)).min(240_000)) } @@ -1568,7 +1599,7 @@ pub(crate) async fn finish_lastfm(app: tauri::AppHandle) -> Result(); let result = state.lastfm.finish(&app).await?; state.lastfm.set_enabled(true).await; - crate::set_lastfm_scrobbling(&app, true)?; + crate::set_lastfm_scrobbling(&app, true).await?; Ok(result) } @@ -1651,6 +1682,31 @@ mod tests { assert!(credentials_from(Some(" key "), Some(" secret ")).is_some()); } + #[tokio::test] + async fn import_owner_guard_rejects_a_different_connected_account() { + let directory = tempfile::tempdir().unwrap(); + let service = Service::new(directory.path(), true, true); + service.runtime.lock().await.session = Some(LastFmSession { + username: "user".into(), + key: "session".into(), + }); + + assert_eq!( + service + .with_import_owner("user", || async { Ok::<_, String>(7) }) + .await + .unwrap(), + Some(7) + ); + assert_eq!( + service + .with_import_owner("other", || async { Ok::<_, String>(7) }) + .await + .unwrap(), + None + ); + } + #[test] fn dev_session_store_round_trips_with_owner_only_permissions() { let directory = tempfile::tempdir().unwrap(); @@ -1703,6 +1759,19 @@ mod tests { ); } + #[test] + fn recent_tracks_import_params_keep_the_fixed_cutoff_and_page_limit() { + assert_eq!( + import_recent_tracks_params("last.fm-user", 7, 1786804381), + vec![ + ("user".into(), "last.fm-user".into()), + ("page".into(), "7".into()), + ("limit".into(), "200".into()), + ("to".into(), "1786804381".into()), + ] + ); + } + #[test] fn metadata_filters_unknown_artist_and_album() { assert!(Scrobble::from_track(&track("Unknown Artist", "Album"), 1).is_none()); diff --git a/apps/desktop/src-tauri/src/lastfm_import.rs b/apps/desktop/src-tauri/src/lastfm_import.rs index 5f28f33..050355f 100644 --- a/apps/desktop/src-tauri/src/lastfm_import.rs +++ b/apps/desktop/src-tauri/src/lastfm_import.rs @@ -1,12 +1,13 @@ use std::{ - collections::{BTreeMap, BTreeSet}, + collections::{BTreeMap, BTreeSet, HashMap}, fs, + future::Future, path::{Path, PathBuf}, sync::{ atomic::{AtomicBool, Ordering}, Arc, }, - time::{Duration, SystemTime, UNIX_EPOCH}, + time::{SystemTime, UNIX_EPOCH}, }; use retune_core::model::{AlbumKey, Library, Rating, TrackEdit}; @@ -15,15 +16,18 @@ use serde_json::Value; use tauri::{Emitter, Manager, WebviewUrl, WebviewWindowBuilder}; use tokio::sync::Mutex; -pub(crate) const SESSION_VERSION: u8 = 1; +pub(crate) const SESSION_VERSION: u8 = 2; pub(crate) const LASTFM_PAGE_LIMIT: u32 = 200; +pub(crate) const LASTFM_REVIEW_BATCH_SIZE: usize = 100; +const LASTFM_QUEUE_PAGE_LIMIT: usize = LASTFM_REVIEW_BATCH_SIZE; pub(crate) const MAX_SERIALIZED_SESSION_BYTES: usize = 100 * 1024 * 1024; +const MAX_RAW_CACHE_BYTES: u64 = 100 * 1024 * 1024; #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub(crate) enum ImportPhase { Downloading, - Matching, + Aggregating, Review, Done, Suspended, @@ -227,13 +231,15 @@ pub(crate) struct ImportBatch { #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub(crate) struct LastFmImportSessionV1 { +pub(crate) struct LastFmImportSessionV2 { pub version: u8, pub lastfm_username: String, - pub spotify_account_id: String, - pub snapshot_to: u64, + pub spotify_account_id: Option, + pub history_to: u64, + pub cache_id: String, pub next_page: u32, pub total_pages: Option, + pub downloaded_pages: u32, pub total_scrobbles: u64, pub included_scrobbles: u64, pub skipped_now_playing: u64, @@ -258,10 +264,9 @@ pub(crate) struct ImportStateView { pub spotify_account_id: Option, pub next_page: u32, pub total_pages: Option, + pub downloaded_pages: u32, pub total_scrobbles: u64, pub included_scrobbles: u64, - pub matched_rows: usize, - pub match_total: usize, pub defaults: ImportDefaults, pub remaining: usize, pub retryable_error: Option, @@ -281,17 +286,34 @@ pub(crate) enum QueueStatus { #[derive(Clone, Debug, Eq, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub(crate) struct ImportQueueItem { + pub page: u32, pub artist: String, pub album: String, pub play_count: u64, pub latest: u64, - pub source_ids: Vec, + pub source_count: usize, pub remaining: bool, pub album_entities: u32, pub track_entities: u32, pub status: Option, } +#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct ImportQueuePage { + pub items: Vec, + pub cursor: usize, + pub next_cursor: Option, + pub total: usize, +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct AcceptAllSummary { + pub album_entities: u32, + pub track_entities: u32, +} + #[derive(Clone, Debug, Eq, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub(crate) struct ImportPageItem { @@ -304,6 +326,7 @@ pub(crate) struct ImportPageItem { #[serde(rename_all = "camelCase")] pub(crate) struct ImportPageView { pub state: ImportStateView, + pub batch_id: u32, pub artist: String, pub album: String, pub page_number: usize, @@ -315,34 +338,34 @@ pub(crate) struct ImportPageView { pub locked_count_modes: BTreeSet, } -impl LastFmImportSessionV1 { - #[cfg_attr(not(test), allow(dead_code))] +impl LastFmImportSessionV2 { + #[cfg(test)] pub(crate) fn new( lastfm_username: String, spotify_account_id: String, - snapshot_to: u64, + history_to: u64, ) -> Self { - Self::new_with_defaults( - lastfm_username, - spotify_account_id, - snapshot_to, - ImportDefaults::default(), - ) + let mut session = + Self::new_with_defaults(lastfm_username, history_to, ImportDefaults::default()); + session.spotify_account_id = Some(spotify_account_id); + session } pub(crate) fn new_with_defaults( lastfm_username: String, - spotify_account_id: String, - snapshot_to: u64, + history_to: u64, defaults: ImportDefaults, ) -> Self { + let cache_id = snapshot_cache_id(&lastfm_username, history_to); Self { version: SESSION_VERSION, lastfm_username, - spotify_account_id, - snapshot_to, + spotify_account_id: None, + history_to, + cache_id, next_page: 1, total_pages: None, + downloaded_pages: 0, total_scrobbles: 0, included_scrobbles: 0, skipped_now_playing: 0, @@ -400,9 +423,52 @@ impl LastFmImportSessionV1 { } }) } + + fn options_for_batch(&self, batch_id: u32, artist: &str, album: &str) -> PageOptions { + let Some(batch) = review_batches(self) + .into_iter() + .find(|batch| batch.page == batch_id) + else { + let mut options = self.options_for(artist, album); + options.selected_track_ids.clear(); + return options; + }; + let batch_ids = batch.source_ids.iter().collect::>(); + let batch_options = self.page_options.get(&batch_options_key(batch_id)).cloned(); + let legacy_options = self + .page_options + .get(&format!("{artist}\u{1f}{album}")) + .cloned(); + let mut options = batch_options + .clone() + .or(legacy_options.clone()) + .unwrap_or_else(|| PageOptions::from_defaults(&self.defaults)); + if batch_options.is_some() || legacy_options.is_some() { + options + .selected_track_ids + .retain(|id| batch_ids.contains(id)); + } else { + options.selected_track_ids = batch + .source_ids + .iter() + .filter(|id| { + let id = (*id).as_str(); + self.rows.iter().any(|row| { + row.stable_id == id + && row.artist == artist + && row.album == album + && is_actionable(self, &row.stable_id) + }) + }) + .cloned() + .collect(); + } + options + } } -#[derive(Clone, Debug, Default, Eq, PartialEq)] +#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] pub(crate) struct ParsedRecentTracksPage { pub page: u32, pub total_pages: Option, @@ -412,7 +478,8 @@ pub(crate) struct ParsedRecentTracksPage { pub skipped_undated: u64, } -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] pub(crate) struct ParsedScrobble { pub artist: String, pub album: String, @@ -482,6 +549,27 @@ pub(crate) fn parse_recent_tracks_page(value: &Value) -> Result Option<&str> { value.as_str() } @@ -530,12 +618,30 @@ fn source_id(artist: &str, album: &str, track: &str) -> String { ) } +fn snapshot_cache_id(username: &str, history_to: u64) -> String { + format!( + "{}-{history_to}", + username + .as_bytes() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::() + ) +} + pub(crate) fn aggregate_scrobbles(rows: &mut Vec, scrobbles: &[ParsedScrobble]) { + let mut row_indices = HashMap::with_capacity(rows.len()); + for (index, row) in rows.iter().enumerate() { + row_indices.entry(row.stable_id.clone()).or_insert(index); + } for scrobble in scrobbles { let id = source_id(&scrobble.artist, &scrobble.album, &scrobble.track); - let Some(row) = rows.iter_mut().find(|row| row.stable_id == id) else { + let index = if let Some(index) = row_indices.get(&id).copied() { + index + } else { + let index = rows.len(); rows.push(SourceRow { - stable_id: id, + stable_id: id.clone(), artist: scrobble.artist.clone(), album: scrobble.album.clone(), track: scrobble.track.clone(), @@ -544,11 +650,10 @@ pub(crate) fn aggregate_scrobbles(rows: &mut Vec, scrobbles: &[Parsed earliest: scrobble.timestamp, latest: scrobble.timestamp, }); - let row = rows.last_mut().expect("row was just pushed"); - add_variant(row, scrobble); - continue; + row_indices.insert(id, index); + index }; - add_variant(row, scrobble); + add_variant(&mut rows[index], scrobble); } } @@ -576,6 +681,85 @@ fn add_variant(row: &mut SourceRow, scrobble: &ParsedScrobble) { }); } +fn batch_options_key(batch_id: u32) -> String { + format!("batch:{batch_id}") +} + +fn build_review_batches(rows: &[SourceRow]) -> Vec { + let mut grouped = BTreeMap::<(String, String), Vec>::new(); + for row in rows { + grouped + .entry((row.artist.clone(), row.album.clone())) + .or_default() + .push(row.stable_id.clone()); + } + let mut page = 1; + let mut batches = Vec::new(); + for source_ids in grouped.into_values() { + for chunk in source_ids.chunks(LASTFM_REVIEW_BATCH_SIZE) { + batches.push(ImportBatch { + page, + source_ids: chunk.to_vec(), + }); + page += 1; + } + } + batches +} + +fn review_batches(session: &LastFmImportSessionV2) -> Vec { + if session.batches.is_empty() || session.batches.iter().any(|batch| batch.page == 0) { + build_review_batches(&session.rows) + } else { + session.batches.clone() + } +} + +fn source_row_map(session: &LastFmImportSessionV2) -> HashMap<&str, &SourceRow> { + session + .rows + .iter() + .map(|row| (row.stable_id.as_str(), row)) + .collect() +} + +fn source_batch_map(session: &LastFmImportSessionV2) -> HashMap { + let mut result = HashMap::new(); + for batch in review_batches(session) { + for source_id in &batch.source_ids { + result.insert(source_id.clone(), batch.page); + } + } + result +} + +fn batch_rows<'a>( + batch: &ImportBatch, + rows: &HashMap<&'a str, &'a SourceRow>, +) -> Vec<&'a SourceRow> { + batch + .source_ids + .iter() + .filter_map(|id| rows.get(id.as_str()).copied()) + .collect() +} + +fn requested_batch( + session: &LastFmImportSessionV2, + batch_id: u32, + artist: &str, + album: &str, +) -> Option { + let rows = source_row_map(session); + review_batches(session).into_iter().find(|batch| { + batch.page == batch_id + && batch_rows(batch, &rows).len() == batch.source_ids.len() + && batch_rows(batch, &rows) + .iter() + .all(|row| row.artist == artist && row.album == album) + }) +} + pub(crate) fn resolved_play_count(rows: &[&SourceRow], mode: CountMode) -> u64 { match mode { CountMode::Sum => rows @@ -696,16 +880,39 @@ pub(crate) fn apply_metadata( #[derive(Clone)] pub(crate) struct ImportSessionStore { path: PathBuf, + cache_root: PathBuf, +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct RawCacheManifest { + version: u8, + cache_id: String, + lastfm_username: String, + history_to: u64, + total_pages: u32, + pages: BTreeMap, +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct CachedRawPage { + lastfm_username: String, + history_to: u64, + total_pages: u32, + parsed: ParsedRecentTracksPage, } impl ImportSessionStore { pub(crate) fn new(app_data_dir: impl AsRef) -> Self { + let app_data_dir = app_data_dir.as_ref(); Self { - path: app_data_dir.as_ref().join("lastfm-import.json"), + path: app_data_dir.join("lastfm-import.json"), + cache_root: app_data_dir.join("lastfm-import-cache"), } } - pub(crate) fn load(&self) -> Result, String> { + pub(crate) fn load(&self) -> Result, String> { let bytes = match fs::read(&self.path) { Ok(bytes) => bytes, Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), @@ -713,9 +920,10 @@ impl ImportSessionStore { }; if bytes.len() > MAX_SERIALIZED_SESSION_BYTES { self.quarantine()?; + self.quarantine_cache_root()?; return Ok(None); } - let parsed = serde_json::from_slice::(&bytes); + let parsed = serde_json::from_slice::(&bytes); match parsed { Ok(session) if session.version == SESSION_VERSION @@ -725,16 +933,27 @@ impl ImportSessionStore { .values() .all(|options| options.validate().is_ok()) => { + if (matches!( + session.phase, + ImportPhase::Downloading | ImportPhase::Aggregating + ) || suspended_source_phase(&session)) + && self.validate_cache(&session).is_err() + { + self.quarantine_snapshot(&session.cache_id)?; + self.quarantine()?; + return Ok(None); + } Ok(Some(session)) } Ok(_) | Err(_) => { self.quarantine()?; + self.quarantine_cache_root()?; Ok(None) } } } - pub(crate) fn save(&self, session: &LastFmImportSessionV1) -> Result<(), String> { + pub(crate) fn save(&self, session: &LastFmImportSessionV2) -> Result<(), String> { let bytes = serde_json::to_vec(session) .map_err(|_| "Could not serialize the Last.fm import session.".to_string())?; if bytes.len() > MAX_SERIALIZED_SESSION_BYTES { @@ -743,6 +962,251 @@ impl ImportSessionStore { super::lastfm::atomic_write(&self.path, &bytes, true) } + fn cache_path(&self, cache_id: &str) -> PathBuf { + self.cache_root.join(cache_id) + } + + fn manifest_path(&self, cache_id: &str) -> PathBuf { + self.cache_path(cache_id).join("manifest.json") + } + + fn page_path(&self, cache_id: &str, page: u32) -> PathBuf { + self.cache_path(cache_id).join(format!("page-{page}.json")) + } + + fn read_manifest( + &self, + session: &LastFmImportSessionV2, + ) -> Result, String> { + let path = self.manifest_path(&session.cache_id); + let metadata = match fs::metadata(&path) { + Ok(metadata) => metadata, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(_) => return Err("Could not read the Last.fm import cache manifest.".into()), + }; + if metadata.len() > MAX_RAW_CACHE_BYTES { + return Err( + "The Last.fm import cache manifest exceeds the 100 MB safety limit.".into(), + ); + } + let bytes = fs::read(path) + .map_err(|_| "Could not read the Last.fm import cache manifest.".to_string())?; + serde_json::from_slice(&bytes) + .map(Some) + .map_err(|_| "The Last.fm import cache manifest is corrupt.".into()) + } + + fn write_page( + &self, + session: &LastFmImportSessionV2, + parsed: &ParsedRecentTracksPage, + ) -> Result<(), String> { + let total_pages = session + .total_pages + .ok_or_else(|| "Last.fm import metadata is not available yet.".to_string())?; + if parsed.page == 0 || parsed.page > total_pages { + return Err("Last.fm import page metadata is out of range.".into()); + } + let cached = CachedRawPage { + lastfm_username: session.lastfm_username.clone(), + history_to: session.history_to, + total_pages, + parsed: parsed.clone(), + }; + let bytes = serde_json::to_vec(&cached) + .map_err(|_| "Could not serialize a Last.fm import page.".to_string())?; + if bytes.len() as u64 > MAX_RAW_CACHE_BYTES { + return Err("The Last.fm import page exceeds the 100 MB safety limit.".into()); + } + + let manifest = self.read_manifest(session)?; + if let Some(manifest) = &manifest { + self.validate_manifest_metadata(manifest, session, total_pages)?; + } + let mut manifest = manifest.unwrap_or_else(|| RawCacheManifest { + version: SESSION_VERSION, + cache_id: session.cache_id.clone(), + lastfm_username: session.lastfm_username.clone(), + history_to: session.history_to, + total_pages, + pages: BTreeMap::new(), + }); + let previous = manifest.pages.insert(parsed.page, bytes.len() as u64); + let current_size = manifest + .pages + .values() + .copied() + .try_fold(0_u64, u64::checked_add) + .ok_or_else(|| "The Last.fm import cache size is invalid.".to_string())?; + if current_size > MAX_RAW_CACHE_BYTES { + match previous { + Some(previous) => { + manifest.pages.insert(parsed.page, previous); + } + None => { + manifest.pages.remove(&parsed.page); + } + } + return Err("The Last.fm import cache exceeds the 100 MB safety limit.".into()); + } + fs::create_dir_all(self.cache_path(&session.cache_id)) + .map_err(|_| "Could not create the Last.fm import cache.".to_string())?; + super::lastfm::atomic_write( + &self.page_path(&session.cache_id, parsed.page), + &bytes, + true, + )?; + let manifest_bytes = serde_json::to_vec(&manifest) + .map_err(|_| "Could not serialize the Last.fm import cache manifest.".to_string())?; + super::lastfm::atomic_write( + &self.manifest_path(&session.cache_id), + &manifest_bytes, + true, + ) + } + + fn validate_cache(&self, session: &LastFmImportSessionV2) -> Result<(), String> { + validate_session_cursor(session)?; + let Some(manifest) = self.read_manifest(session)? else { + return if session.downloaded_pages == 0 { + Ok(()) + } else { + Err("The Last.fm import cache manifest is missing.".into()) + }; + }; + let total_pages = session + .total_pages + .ok_or_else(|| "The Last.fm import cache has no page total.".to_string())?; + self.validate_manifest_metadata(&manifest, session, total_pages)?; + if session.next_page < total_pages { + for page in (session.next_page + 1)..=total_pages { + if !manifest.pages.contains_key(&page) { + return Err("The Last.fm import cache is missing an acknowledged page.".into()); + } + } + } + let total_size = manifest + .pages + .values() + .copied() + .try_fold(0_u64, u64::checked_add) + .ok_or_else(|| "The Last.fm import cache size is invalid.".to_string())?; + if total_size > MAX_RAW_CACHE_BYTES { + return Err("The Last.fm import cache exceeds the 100 MB safety limit.".into()); + } + for (&page, &recorded_size) in &manifest.pages { + if page == 0 || page > total_pages { + return Err("The Last.fm import cache contains an invalid page.".into()); + } + let path = self.page_path(&session.cache_id, page); + let actual_size = fs::metadata(&path) + .map_err(|_| "An acknowledged Last.fm import page is missing.".to_string())? + .len(); + if actual_size != recorded_size || recorded_size > MAX_RAW_CACHE_BYTES { + return Err( + "An acknowledged Last.fm import page is oversized or truncated.".into(), + ); + } + let bytes = fs::read(&path) + .map_err(|_| "An acknowledged Last.fm import page is missing.".to_string())?; + let cached = serde_json::from_slice::(&bytes) + .map_err(|_| "An acknowledged Last.fm import page is corrupt.".to_string())?; + if cached.lastfm_username != session.lastfm_username + || cached.history_to != session.history_to + || cached.total_pages != total_pages + || cached.parsed.page != page + || cached + .parsed + .total_pages + .is_some_and(|value| value != total_pages) + { + return Err("An acknowledged Last.fm import page has mismatched metadata.".into()); + } + } + Ok(()) + } + + fn validate_manifest_metadata( + &self, + manifest: &RawCacheManifest, + session: &LastFmImportSessionV2, + total_pages: u32, + ) -> Result<(), String> { + if manifest.version != SESSION_VERSION + || manifest.cache_id != session.cache_id + || manifest.lastfm_username != session.lastfm_username + || manifest.history_to != session.history_to + || manifest.total_pages != total_pages + { + return Err("The Last.fm import cache metadata does not match its session.".into()); + } + Ok(()) + } + + fn read_pages(&self, session: &LastFmImportSessionV2) -> Result, String> { + self.validate_cache(session)?; + let Some(manifest) = self.read_manifest(session)? else { + return Ok(Vec::new()); + }; + let mut scrobbles = Vec::new(); + for page in manifest.pages.keys() { + let bytes = fs::read(self.page_path(&session.cache_id, *page)) + .map_err(|_| "An acknowledged Last.fm import page is missing.".to_string())?; + let cached = serde_json::from_slice::(&bytes) + .map_err(|_| "An acknowledged Last.fm import page is corrupt.".to_string())?; + if cached.lastfm_username != session.lastfm_username { + return Err( + "An acknowledged Last.fm import page belongs to another Last.fm account." + .into(), + ); + } + scrobbles.extend( + cached + .parsed + .tracks + .into_iter() + .filter(|scrobble| scrobble.timestamp < session.history_to), + ); + } + Ok(scrobbles) + } + + fn remove_snapshot(&self, cache_id: &str) { + let _ = fs::remove_dir_all(self.cache_path(cache_id)); + } + + fn quarantine_snapshot(&self, cache_id: &str) -> Result<(), String> { + let path = self.cache_path(cache_id); + if !path.exists() { + return Ok(()); + } + let stamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + fs::rename( + &path, + path.with_file_name(format!("{cache_id}.quarantine-{stamp}")), + ) + .map_err(|_| "Could not quarantine the Last.fm import cache.".to_string()) + } + + fn quarantine_cache_root(&self) -> Result<(), String> { + if !self.cache_root.exists() { + return Ok(()); + } + let stamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + fs::rename( + &self.cache_root, + self.cache_root + .with_file_name(format!("lastfm-import-cache.quarantine-{stamp}")), + ) + .map_err(|_| "Could not quarantine the Last.fm import cache.".to_string()) + } + fn quarantine(&self) -> Result<(), String> { let stamp = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -754,9 +1218,37 @@ impl ImportSessionStore { } } +fn validate_session_cursor(session: &LastFmImportSessionV2) -> Result<(), String> { + match session.total_pages { + None if session.next_page == 1 && session.downloaded_pages == 0 => Ok(()), + None => Err("The Last.fm import cursor has no valid page total.".into()), + Some(0) if session.next_page == 0 && session.downloaded_pages == 0 => Ok(()), + Some(total_pages) + if session.downloaded_pages <= total_pages + && session.next_page == total_pages.saturating_sub(session.downloaded_pages) => + { + Ok(()) + } + Some(_) => Err("The Last.fm import cursor is inconsistent with its page total.".into()), + } +} + +fn suspended_source_phase(session: &LastFmImportSessionV2) -> bool { + session.phase == ImportPhase::Suspended && session.rows.is_empty() +} + +fn requires_spotify_ownership(session: &LastFmImportSessionV2) -> bool { + session.spotify_account_id.is_some() + && matches!( + session.phase, + ImportPhase::Review | ImportPhase::Done | ImportPhase::Suspended + ) +} + pub(crate) struct Service { store: ImportSessionStore, - session: Mutex>, + session: Mutex>, + lazy_match_lock: Mutex<()>, running: AtomicBool, } @@ -773,31 +1265,25 @@ impl Service { Arc::new(Self { store, session: Mutex::new(session), + lazy_match_lock: Mutex::new(()), running: AtomicBool::new(false), }) } pub(crate) async fn state(&self) -> ImportStateView { - self.state_with_identity(None).await - } - - pub(crate) async fn state_with_identity( - &self, - identity: Option<(String, String)>, - ) -> ImportStateView { let session = self.session.lock().await; match session.as_ref() { Some(session) if session.phase == ImportPhase::Suspended => suspended_state_view(), Some(session) => state_view(Some(session)), - None => state_view_with_identity(None, identity.as_ref()), + None => state_view(None), } } - async fn snapshot(&self) -> Option { + async fn snapshot(&self) -> Option { self.session.lock().await.clone() } - async fn persist(&self, session: LastFmImportSessionV1) -> Result<(), String> { + async fn persist(&self, session: LastFmImportSessionV2) -> Result<(), String> { let store = self.store.clone(); tauri::async_runtime::spawn_blocking(move || store.save(&session)) .await @@ -805,15 +1291,15 @@ impl Service { } #[cfg(test)] - async fn save(&self, session: LastFmImportSessionV1) -> Result<(), String> { + async fn save(&self, session: LastFmImportSessionV2) -> Result<(), String> { self.mutate_session(|_| Ok((Some(session), ()))).await } async fn mutate_session(&self, mutation: F) -> Result where F: FnOnce( - Option, - ) -> Result<(Option, R), String>, + Option, + ) -> Result<(Option, R), String>, { let mut current = self.session.lock().await; let (next, result) = mutation(current.clone())?; @@ -832,14 +1318,14 @@ impl Service { mutation: F, ) -> Result where - F: FnOnce(LastFmImportSessionV1) -> Result<(LastFmImportSessionV1, R), String>, + F: FnOnce(LastFmImportSessionV2) -> Result<(LastFmImportSessionV2, R), String>, { self.mutate_session(|session| { let Some(session) = session else { return Err("No Last.fm import session is active.".into()); }; if session.lastfm_username != username - || session.spotify_account_id != spotify_account_id + || session.spotify_account_id.as_deref() != Some(spotify_account_id) || !allowed_phase(session.phase) { return Err( @@ -871,20 +1357,48 @@ impl Service { pub(crate) async fn start_or_resume( &self, username: &str, - spotify_account_id: &str, - snapshot_to: u64, + history_to: u64, defaults: Option, ) -> Result { if let Some(defaults) = &defaults { defaults.validate()?; } + if let Some(session) = self.snapshot().await { + if suspended_source_phase(&session) { + let store = self.store.clone(); + let validation_session = session.clone(); + let cache_valid = tauri::async_runtime::spawn_blocking(move || { + store.validate_cache(&validation_session).is_ok() + }) + .await + .map_err(|_| "Last.fm import cache validation task stopped.".to_string())?; + if !cache_valid { + let mut current = self.session.lock().await; + let same_source = current.as_ref().is_some_and(|current| { + suspended_source_phase(current) + && current.cache_id == session.cache_id + && current.lastfm_username == session.lastfm_username + && current.history_to == session.history_to + }); + if same_source { + let store = self.store.clone(); + let cache_id = session.cache_id.clone(); + tauri::async_runtime::spawn_blocking(move || { + store.quarantine_snapshot(&cache_id)?; + store.quarantine() + }) + .await + .map_err(|_| "Last.fm import quarantine task stopped.".to_string())??; + *current = None; + } + } + } + } let result = self .mutate_session(|current| { let session = match current { Some(mut session) => { - if session.lastfm_username != username - || session.spotify_account_id != spotify_account_id - { + if session.lastfm_username != username { session.phase = ImportPhase::Suspended; session.retryable_error = Some(RetryableError { message: "This import is suspended because the connected account changed. Reconnect Last.fm and Spotify to resume.".into(), @@ -899,9 +1413,15 @@ impl Service { if session.phase == ImportPhase::Suspended { session.phase = if session .total_pages - .is_some_and(|total_pages| session.next_page > total_pages) + .is_some_and(|total_pages| { + total_pages == 0 || session.downloaded_pages >= total_pages + }) { - ImportPhase::Matching + if session.rows.is_empty() { + ImportPhase::Aggregating + } else { + ImportPhase::Review + } } else { ImportPhase::Downloading }; @@ -909,12 +1429,13 @@ impl Service { } session } - None => LastFmImportSessionV1::new_with_defaults( - username.to_owned(), - spotify_account_id.to_owned(), - snapshot_to, - defaults.unwrap_or_default(), - ), + None => { + LastFmImportSessionV2::new_with_defaults( + username.to_owned(), + history_to, + defaults.unwrap_or_default(), + ) + } }; let view = state_view(Some(&session)); Ok((Some(session), Ok(view))) @@ -923,11 +1444,82 @@ impl Service { result } + async fn set_metadata( + &self, + total_pages: u32, + total_scrobbles: u64, + ) -> Result { + self.mutate_session(|current| { + let Some(mut session) = current else { + return Err("No Last.fm import session is active.".into()); + }; + if session.phase != ImportPhase::Downloading { + return Ok((Some(session.clone()), state_view(Some(&session)))); + } + if let Some(existing) = session.total_pages { + if existing != total_pages { + return Err("Last.fm import metadata changed during the snapshot.".into()); + } + return Ok((Some(session.clone()), state_view(Some(&session)))); + } + session.total_pages = Some(total_pages); + session.total_scrobbles = total_scrobbles; + session.next_page = total_pages; + session.retryable_error = None; + if total_pages == 0 { + session.phase = ImportPhase::Aggregating; + } + Ok((Some(session.clone()), state_view(Some(&session)))) + }) + .await + } + async fn checkpoint_page( &self, page: u32, parsed: &ParsedRecentTracksPage, ) -> Result { + let Some(before) = self.snapshot().await else { + return Err("No Last.fm import session is active.".into()); + }; + if before.phase != ImportPhase::Downloading { + return Ok(state_view(Some(&before))); + } + if parsed.page != page { + return Err(format!( + "Last.fm response was for page {}, expected page {page}.", + parsed.page + )); + } + let total_pages = before + .total_pages + .or(parsed.total_pages) + .ok_or_else(|| "Last.fm import metadata is not available yet.".to_string())?; + if parsed.total_pages.is_some_and(|value| value != total_pages) { + return Err("Last.fm page metadata changed during the snapshot.".into()); + } + let expected_page = if before.next_page == 0 { + page + } else { + before.next_page + }; + if page != expected_page { + if page < expected_page { + return Err("Last.fm import pages must be checkpointed sequentially.".into()); + } + return Ok(state_view(Some(&before))); + } + let mut cache_session = before.clone(); + cache_session.total_pages = Some(total_pages); + let mut filtered = parsed.clone(); + discard_post_cutoff(&mut filtered, before.history_to); + let store = self.store.clone(); + let cached_page = filtered.clone(); + tauri::async_runtime::spawn_blocking(move || { + store.write_page(&cache_session, &cached_page) + }) + .await + .map_err(|_| "Last.fm import cache task stopped.".to_string())??; let result = self .mutate_session(|current| { let Some(mut session) = current else { @@ -936,48 +1528,25 @@ impl Service { if session.phase != ImportPhase::Downloading { return Ok((Some(session.clone()), state_view(Some(&session)))); } - if parsed.page != page { - return Err(format!( - "Last.fm response was for page {}, expected page {page}.", - parsed.page - )); - } - if page < session.next_page { - return Ok((Some(session.clone()), state_view(Some(&session)))); - } - if page > session.next_page { - return Err("Last.fm import pages must be checkpointed sequentially.".into()); + if session.next_page != 0 && session.next_page != page { + return Err("Last.fm import cursor changed before page acknowledgement.".into()); } - aggregate_scrobbles(&mut session.rows, &parsed.tracks); - session.total_pages = parsed.total_pages.or(session.total_pages); - session.total_scrobbles = parsed.total.unwrap_or(session.total_scrobbles); + session.total_pages = Some(total_pages); + session.total_scrobbles = filtered.total.unwrap_or(session.total_scrobbles); session.included_scrobbles = session .included_scrobbles - .saturating_add(parsed.tracks.len() as u64); + .saturating_add(filtered.tracks.len() as u64); session.skipped_now_playing = session .skipped_now_playing - .saturating_add(parsed.skipped_now_playing); + .saturating_add(filtered.skipped_now_playing); session.skipped_undated = session .skipped_undated - .saturating_add(parsed.skipped_undated); - session.batches.push(ImportBatch { - page, - source_ids: parsed - .tracks - .iter() - .map(|track| source_id(&track.artist, &track.album, &track.track)) - .collect::>() - .into_iter() - .collect(), - }); - session.next_page = page.saturating_add(1); - if session - .total_pages - .is_some_and(|total_pages| session.next_page > total_pages) - || (parsed.total_pages.is_none() - && parsed.tracks.len() < LASTFM_PAGE_LIMIT as usize) - { - session.phase = ImportPhase::Matching; + .saturating_add(filtered.skipped_undated); + session.downloaded_pages = session.downloaded_pages.saturating_add(1); + session.next_page = page.saturating_sub(1); + if session.downloaded_pages >= total_pages { + session.next_page = 0; + session.phase = ImportPhase::Aggregating; } session.retryable_error = None; Ok((Some(session.clone()), state_view(Some(&session)))) @@ -986,66 +1555,141 @@ impl Service { Ok(result) } - async fn set_retryable_error(&self, error: RetryableError) -> Result<(), String> { - self.mutate_session(|session| { - let Some(mut session) = session else { - return Ok((None, ())); - }; - if session.phase == ImportPhase::Suspended { - return Ok((Some(session), ())); - } - session.retryable_error = Some(error); - Ok((Some(session), ())) - }) - .await - } - - async fn set_match( + async fn aggregate_cached( &self, - username: &str, - spotify_account_id: &str, - result: MatchResult, - ) -> Result<(), String> { - self.set_matches(username, spotify_account_id, vec![result]) + lastfm: Option<&crate::lastfm::Service>, + ) -> Result { + let Some(session) = self.snapshot().await else { + return Err("No Last.fm import session is active.".into()); + }; + if session.phase != ImportPhase::Aggregating { + return Ok(state_view(Some(&session))); + } + let store = self.store.clone(); + let blocking_session = session.clone(); + let aggregation = tauri::async_runtime::spawn_blocking(move || { + let mut scrobbles = store.read_pages(&blocking_session)?; + sort_scrobbles(&mut scrobbles); + let mut rows = Vec::new(); + aggregate_scrobbles(&mut rows, &scrobbles); + let batches = build_review_batches(&rows); + Ok::<_, String>((rows, batches)) + }) + .await + .map_err(|_| "Last.fm import aggregation task stopped.".to_string())?; + let (rows, batches) = match aggregation { + Ok(result) => result, + Err(error) => { + self.invalidate_snapshot().await?; + return Err(error); + } + }; + let commit = || async { + self.mutate_session(|current| { + let Some(mut current) = current else { + return Err("No Last.fm import session is active.".into()); + }; + if current.cache_id != session.cache_id || current.phase != ImportPhase::Aggregating + { + return Err("Last.fm import changed while aggregation was running.".into()); + } + current.rows = rows; + current.batches = batches; + current.phase = if current.rows.is_empty() { + ImportPhase::Done + } else { + ImportPhase::Review + }; + current.retryable_error = None; + Ok((Some(current.clone()), state_view(Some(¤t)))) + }) .await + }; + let result = match lastfm { + Some(lastfm) => match lastfm + .with_import_owner(&session.lastfm_username, commit) + .await? + { + Some(result) => result, + None => { + self.suspend_for_account_mismatch().await?; + return Ok(self.state().await); + } + }, + None => commit().await?, + }; + self.store.remove_snapshot(&session.cache_id); + Ok(result) } - async fn set_matches( + async fn invalidate_snapshot(&self) -> Result<(), String> { + let mut current = self.session.lock().await; + if let Some(session) = current.as_ref() { + self.store.quarantine_snapshot(&session.cache_id)?; + self.store.quarantine()?; + } + *current = None; + Ok(()) + } + + async fn set_retryable_error(&self, error: Option) -> Result<(), String> { + self.mutate_session(|session| { + let Some(mut session) = session else { + return Ok((None, ())); + }; + if session.phase == ImportPhase::Suspended { + return Ok((Some(session), ())); + } + session.retryable_error = error; + Ok((Some(session), ())) + }) + .await + } + + async fn set_match( &self, username: &str, spotify_account_id: &str, - results: Vec, + batch_id: u32, + result: MatchResult, ) -> Result<(), String> { - self.mutate_owned_session( - username, - spotify_account_id, - review_phase_allowed, - |mut session| { - for result in results { - session.matches.insert(result.source_id.clone(), result); - } - Ok((session, ())) - }, - ) - .await + self.set_matches(username, spotify_account_id, batch_id, vec![result]) + .await } - async fn set_matches_during_matching( + async fn set_matches( &self, username: &str, spotify_account_id: &str, + batch_id: u32, results: Vec, ) -> Result<(), String> { self.mutate_session(|session| { let Some(mut session) = session else { return Err("No Last.fm import session is active.".into()); }; - if session.phase != ImportPhase::Matching - || session.lastfm_username != username - || session.spotify_account_id != spotify_account_id + if session.lastfm_username != username + || (session.spotify_account_id.is_some() + && session.spotify_account_id.as_deref() != Some(spotify_account_id)) + || !review_phase_allowed(session.phase) + { + return Err( + "The Last.fm import is no longer active for this account or phase.".into(), + ); + } + let Some(batch) = review_batches(&session) + .into_iter() + .find(|batch| batch.page == batch_id) + else { + return Err("Unknown Last.fm import review batch.".into()); + }; + if results + .iter() + .any(|result| !batch.source_ids.iter().any(|id| id == &result.source_id)) { - return Err("Last.fm matching stopped because the connected account or import phase changed.".into()); + return Err("A match does not belong to this review batch.".into()); } + session.spotify_account_id = Some(spotify_account_id.to_owned()); for result in results { session.matches.insert(result.source_id.clone(), result); } @@ -1101,136 +1745,94 @@ impl Service { .await } - async fn finish_matching_if_current( + pub(crate) async fn queue_page( &self, - username: &str, - spotify_account_id: &str, - ) -> Result<(), String> { - self.mutate_session(|session| { - let Some(mut session) = session else { - return Ok((None, ())); - }; - if session.phase != ImportPhase::Matching - || session.lastfm_username != username - || session.spotify_account_id != spotify_account_id - { - return Err("Last.fm matching stopped because the connected account or import phase changed.".into()); - } - session.phase = ImportPhase::Review; - session.retryable_error = None; - Ok((Some(session), ())) - }) - .await - } - - pub(crate) async fn queue(&self) -> Vec { + cursor: usize, + limit: usize, + ) -> Result { + if limit == 0 || limit > LASTFM_QUEUE_PAGE_LIMIT { + return Err(format!( + "Last.fm import queue limit must be between 1 and {LASTFM_QUEUE_PAGE_LIMIT}." + )); + } let Some(session) = self.snapshot().await else { - return Vec::new(); + return Ok(ImportQueuePage { + items: Vec::new(), + cursor, + next_cursor: None, + total: 0, + }); }; if session.phase == ImportPhase::Suspended { - return Vec::new(); - } - let mut grouped = BTreeMap::<(String, String), Vec<&SourceRow>>::new(); - for row in &session.rows { - grouped - .entry((row.artist.clone(), row.album.clone())) - .or_default() - .push(row); + return Ok(ImportQueuePage { + items: Vec::new(), + cursor, + next_cursor: None, + total: 0, + }); } - grouped - .into_iter() - .map(|((artist, album), rows)| { - let options = session.options_for(&artist, &album); - let remaining = rows.iter().any(|row| { - let decision = default_decision(&session, &row.stable_id); - matches!(decision.status, RowStatus::Pending | RowStatus::Skipped) - && !decision.excluded - }); - let selected = rows - .iter() - .filter(|row| { - let decision = default_decision(&session, &row.stable_id); - options.selected_track_ids.contains(&row.stable_id) - && matches!(decision.status, RowStatus::Pending | RowStatus::Skipped) - && !decision.excluded - }) - .collect::>(); - let mut album_entities = 0; - let mut track_uris = BTreeSet::new(); - if options.import_content { - if options.whole_album { - album_entities = selected - .iter() - .filter_map(|row| session.matches.get(&row.stable_id)) - .filter_map(|result| { - result.selected_uri.as_deref().or_else(|| { - best_candidate(result).map(|candidate| candidate.uri.as_str()) - }) - }) - .any(|uri| uri.starts_with("spotify:album:")) - as u32; - } else { - for row in &selected { - if let Some(result) = session.matches.get(&row.stable_id) { - if let Some(uri) = matched_track_uri_for_row(result, row) { - track_uris.insert(uri); - } - } - } - } - } - ImportQueueItem { - artist, - album, - play_count: rows - .iter() - .map(|row| row.play_count) - .fold(0, u64::saturating_add), - latest: rows.iter().map(|row| row.latest).max().unwrap_or_default(), - source_ids: rows.iter().map(|row| row.stable_id.clone()).collect(), - remaining, - album_entities, - track_entities: track_uris.len() as u32, - status: queue_status(&session, &rows), + let rows_by_id = source_row_map(&session); + let mut items = Vec::with_capacity(limit); + let mut total = 0; + for batch in review_batches(&session) { + let rows = batch_rows(&batch, &rows_by_id); + if rows.is_empty() { + continue; + } + if total >= cursor && items.len() < limit { + if let Some(item) = queue_item(&session, &batch, &rows) { + items.push(item); } - }) - .collect() + } + total += 1; + } + if cursor > total { + return Err("Last.fm import queue cursor is out of range.".into()); + } + let end = cursor.saturating_add(items.len()).min(total); + Ok(ImportQueuePage { + items, + cursor, + next_cursor: (end < total).then_some(end), + total, + }) } - pub(crate) async fn page(&self, artist: &str, album: &str) -> Option { + pub(crate) async fn page( + &self, + batch_id: u32, + artist: &str, + album: &str, + ) -> Option { let session = self.snapshot().await?; if session.phase == ImportPhase::Suspended { return None; } - let pages = session - .rows - .iter() - .map(|row| (row.artist.clone(), row.album.clone())) - .collect::>(); - let page_number = pages + let batches = review_batches(&session); + let batch = requested_batch(&session, batch_id, artist, album)?; + let rows_by_id = source_row_map(&session); + let rows = batch_rows(&batch, &rows_by_id); + let page_number = batches .iter() - .position(|(page_artist, page_album)| page_artist == artist && page_album == album) - .map(|index| index + 1) - .unwrap_or(1); - let rows = session - .rows + .position(|candidate| candidate.page == batch_id)? + + 1; + let items = rows .iter() - .filter(|row| row.artist == artist && row.album == album) .map(|row| ImportPageItem { - source: row.clone(), + source: (*row).clone(), decision: default_decision(&session, &row.stable_id), match_result: session.matches.get(&row.stable_id).cloned(), }) .collect(); - let options = session.options_for(artist, album); + let options = session.options_for_batch(batch.page, artist, album); let mut fuzzy_groups = BTreeMap::>::new(); - for row in &session.rows { + for row in &rows { let decision = default_decision(&session, &row.stable_id); let participates = !decision.excluded && match decision.status { RowStatus::Done => true, RowStatus::Pending | RowStatus::Skipped => session - .options_for(&row.artist, &row.album) + .options_for_batch(batch.page, artist, album) .selected_track_ids .contains(&row.stable_id), RowStatus::IgnoredAlbum | RowStatus::IgnoredArtist => false, @@ -1248,21 +1850,33 @@ impl Service { fuzzy_groups .entry(target_uri) .or_default() - .push(row.clone()); + .push((*row).clone()); } fuzzy_groups .retain(|_, rows| rows.len() > 1 || rows.iter().any(|row| row.variants.len() > 1)); + let visible_targets = fuzzy_groups.keys().cloned().collect::>(); + let count_modes = session + .count_modes + .iter() + .filter(|(target, _)| visible_targets.contains(*target)) + .map(|(target, mode)| (target.clone(), *mode)) + .collect(); + let locked_count_modes = locked_count_modes(&session) + .into_iter() + .filter(|target| visible_targets.contains(target)) + .collect(); Some(ImportPageView { state: state_view(Some(&session)), + batch_id, artist: artist.to_owned(), album: album.to_owned(), page_number, - page_count: pages.len(), - rows, + page_count: batches.len(), + rows: items, options, fuzzy_groups, - count_modes: session.count_modes.clone(), - locked_count_modes: locked_count_modes(&session), + count_modes, + locked_count_modes, }) } @@ -1270,6 +1884,7 @@ impl Service { &self, username: &str, spotify_account_id: &str, + batch_id: u32, artist: &str, album: &str, options: PageOptions, @@ -1280,19 +1895,24 @@ impl Service { spotify_account_id, review_phase_allowed, |mut session| { + if requested_batch(&session, batch_id, artist, album).is_none() { + return Err("Unknown Last.fm import review batch.".into()); + } session .page_options - .insert(format!("{artist}\u{1f}{album}"), options); + .insert(batch_options_key(batch_id), options); Ok((session, ())) }, ) .await } + #[allow(clippy::too_many_arguments)] async fn review_action( &self, username: &str, spotify_account_id: &str, + batch_id: u32, id: &str, action: &str, artist: &str, @@ -1303,26 +1923,54 @@ impl Service { spotify_account_id, review_phase_allowed, |mut session| { + let Some(batch) = requested_batch(&session, batch_id, artist, album) else { + return Err("Unknown Last.fm import review batch.".into()); + }; + if !batch.source_ids.iter().any(|source_id| source_id == id) { + return Err("The source row does not belong to this review batch.".into()); + } match action { "exclude" | "undo-exclude" => { exclude_row(&mut session, id, action == "exclude"); } - "ignore-album" => ignore_album(&mut session, artist, album), + "ignore-album" => { + for source_id in album_source_ids(&session, artist, album) { + if is_actionable(&session, &source_id) { + session.decisions.insert( + source_id, + RowDecision { + status: RowStatus::IgnoredAlbum, + excluded: false, + }, + ); + } + } + } "ignore-artist" => ignore_artist(&mut session, artist), - "skip-album" => skip_album(&mut session, artist, album), + "skip-album" => { + for source_id in album_source_ids(&session, artist, album) { + if is_actionable(&session, &source_id) { + session.decisions.insert( + source_id, + RowDecision { + status: RowStatus::Skipped, + excluded: false, + }, + ); + } + } + } "restore" => { - let ids = session - .rows - .iter() - .filter(|row| { - row.artist == artist - && row.album == album - && is_actionable(&session, &row.stable_id) - }) - .map(|row| row.stable_id.clone()) - .collect::>(); - for id in ids { - session.decisions.insert(id, RowDecision::default()); + for source_id in album_source_ids(&session, artist, album) { + let decision = default_decision(&session, &source_id); + if !decision.excluded + && matches!( + decision.status, + RowStatus::IgnoredAlbum | RowStatus::Skipped + ) + { + session.decisions.insert(source_id, RowDecision::default()); + } } } _ => return Err("Unknown Last.fm import review action.".into()), @@ -1334,10 +1982,12 @@ impl Service { .await } + #[allow(clippy::too_many_arguments)] async fn commit_rows( &self, username: &str, spotify_account_id: &str, + batch_id: u32, ids: &[String], artist: &str, album: &str, @@ -1348,9 +1998,20 @@ impl Service { spotify_account_id, review_phase_allowed, |mut session| { + let Some(batch) = requested_batch(&session, batch_id, artist, album) else { + return Err("Unknown Last.fm import review batch.".into()); + }; + if ids + .iter() + .any(|id| !batch.source_ids.iter().any(|source_id| source_id == id)) + { + return Err( + "A selected source row does not belong to this review batch.".into(), + ); + } session .page_options - .insert(format!("{artist}\u{1f}{album}"), options); + .insert(batch_options_key(batch_id), options); for id in ids { session.decisions.insert( id.clone(), @@ -1374,6 +2035,7 @@ impl Service { &self, username: &str, spotify_account_id: &str, + batch_id: u32, source_id: &str, uri: &str, ) -> Result<(), String> { @@ -1390,6 +2052,11 @@ impl Service { else { return Err("Unknown Last.fm import source row.".into()); }; + let Some(batch) = requested_batch(&session, batch_id, &row_artist, &row_album) + else { + return Err("The source row does not belong to this review batch.".into()); + }; + let batch_ids = batch.source_ids.iter().cloned().collect::>(); let Some(candidate) = session .matches .get(source_id) @@ -1412,7 +2079,7 @@ impl Service { let related = session .rows .iter() - .filter(|row| row.artist == row_artist && row.album == row_album) + .filter(|row| batch_ids.contains(&row.stable_id)) .map(|row| (row.stable_id.clone(), row.track.clone())) .collect::>(); let mut group_track_matches = BTreeMap::new(); @@ -1436,7 +2103,7 @@ impl Service { for row in session .rows .iter() - .filter(|row| row.artist == row_artist && row.album == row_album) + .filter(|row| batch_ids.contains(&row.stable_id)) { if let Some(result) = session.matches.get_mut(&row.stable_id) { result.track_matches = group_track_matches.clone(); @@ -1459,7 +2126,7 @@ impl Service { let related_ids = session .rows .iter() - .filter(|row| row.artist == row_artist && row.album == row_album) + .filter(|row| batch_ids.contains(&row.stable_id)) .map(|row| row.stable_id.clone()) .collect::>(); let mut group_track_matches = BTreeMap::new(); @@ -1500,42 +2167,28 @@ impl Service { } } -fn state_view(session: Option<&LastFmImportSessionV1>) -> ImportStateView { - state_view_with_identity(session, None) -} - -fn state_view_with_identity( - session: Option<&LastFmImportSessionV1>, - identity: Option<&(String, String)>, -) -> ImportStateView { +fn state_view(session: Option<&LastFmImportSessionV2>) -> ImportStateView { ImportStateView { phase: session.map(|session| session.phase), - username: session - .map(|session| session.lastfm_username.clone()) - .or_else(|| identity.map(|(username, _)| username.clone())), - spotify_account_id: session - .map(|session| session.spotify_account_id.clone()) - .or_else(|| identity.map(|(_, account_id)| account_id.clone())), + username: session.map(|session| session.lastfm_username.clone()), + spotify_account_id: session.and_then(|session| session.spotify_account_id.clone()), next_page: session.map(|session| session.next_page).unwrap_or(1), total_pages: session.and_then(|session| session.total_pages), + downloaded_pages: session + .map(|session| session.downloaded_pages) + .unwrap_or_default(), total_scrobbles: session .map(|session| session.total_scrobbles) .unwrap_or_default(), included_scrobbles: session .map(|session| session.included_scrobbles) .unwrap_or_default(), - matched_rows: session - .map(|session| session.matches.len()) - .unwrap_or_default(), - match_total: session - .map(|session| session.rows.len()) - .unwrap_or_default(), defaults: session .map(|session| session.defaults.clone()) .unwrap_or_default(), remaining: session .filter(|session| matches!(session.phase, ImportPhase::Review | ImportPhase::Done)) - .map(LastFmImportSessionV1::remaining) + .map(LastFmImportSessionV2::remaining) .unwrap_or_default(), retryable_error: session.and_then(|session| session.retryable_error.clone()), search_terms: session.map(|session| session.search_terms).unwrap_or(true), @@ -1549,10 +2202,9 @@ fn suspended_state_view() -> ImportStateView { spotify_account_id: None, next_page: 1, total_pages: None, + downloaded_pages: 0, total_scrobbles: 0, included_scrobbles: 0, - matched_rows: 0, - match_total: 0, defaults: ImportDefaults::default(), remaining: 0, retryable_error: Some(RetryableError { @@ -1564,10 +2216,13 @@ fn suspended_state_view() -> ImportStateView { } } -async fn connected_accounts(app: &tauri::AppHandle) -> Result<(String, String), String> { - let state = app.state::(); - let _membership_guard = state.spotify_library_gate.lock().await; - connected_accounts_locked(&state).await +async fn lastfm_username(app: &tauri::AppHandle) -> Result { + app.state::() + .lastfm + .state() + .await + .username + .ok_or_else(|| "Connect Last.fm before importing its history.".to_string()) } async fn connected_accounts_locked(state: &crate::AppState) -> Result<(String, String), String> { @@ -1615,7 +2270,7 @@ async fn assert_current_account_locked( let Some(session) = service.snapshot().await else { return Err("No Last.fm import session is active.".into()); }; - if session.lastfm_username != username || session.spotify_account_id != spotify_account_id { + if !session_account_matches(&session, &username, &spotify_account_id, true) { service.suspend_for_account_mismatch().await?; return Err( "The saved Last.fm import belongs to a different account; it is suspended for safety." @@ -1632,20 +2287,19 @@ async fn ensure_import_readable(app: &tauri::AppHandle, service: &Service) -> Re let Some(session) = service.snapshot().await else { return Ok(true); }; - if session.phase == ImportPhase::Suspended { - return Ok(false); - } - let state = app.state::(); - let current = { - let _membership_guard = state.spotify_library_gate.lock().await; - connected_accounts_locked(&state).await - }; - match current { - Ok((username, spotify_account_id)) - if username == session.lastfm_username - && spotify_account_id == session.spotify_account_id => - { - Ok(true) + match lastfm_username(app).await { + Ok(username) if username == session.lastfm_username => { + if session.phase == ImportPhase::Suspended { + if requires_spotify_ownership(&session) { + let _ = current_spotify_binding_is_current(app, service, true).await?; + } + return Ok(false); + } + if requires_spotify_ownership(&session) { + current_spotify_binding_is_current(app, service, false).await + } else { + Ok(true) + } } Ok(_) | Err(_) => { service.suspend_for_account_mismatch().await?; @@ -1654,43 +2308,172 @@ async fn ensure_import_readable(app: &tauri::AppHandle, service: &Service) -> Re } } +async fn current_spotify_binding_is_current( + app: &tauri::AppHandle, + service: &Service, + allow_suspended: bool, +) -> Result { + let Some(session) = service.snapshot().await else { + return Ok(false); + }; + if session.spotify_account_id.is_none() { + return Ok(true); + } + let state = app.state::(); + let _membership_guard = state.spotify_library_gate.lock().await; + let expected = session.spotify_account_id.as_deref().unwrap_or_default(); + let cached = state + .spotify_library + .lock() + .expect("Spotify library mutex poisoned") + .clone(); + if cached_spotify_identity_matches(expected, &cached) == Some(false) { + service.suspend_for_account_mismatch().await?; + return Ok(false); + } + let (username, spotify_account_id) = match connected_accounts_locked(&state).await { + Ok(accounts) => accounts, + Err(_) => { + service.suspend_for_account_mismatch().await?; + return Ok(false); + } + }; + if !session_account_matches(&session, &username, &spotify_account_id, true) + || (session.phase == ImportPhase::Suspended && !allow_suspended) + { + service.suspend_for_account_mismatch().await?; + return Ok(false); + } + Ok(true) +} + async fn emit_import_changed( app: &tauri::AppHandle, service: &Service, ) -> Result { - let view = service.state().await; - app.emit("lastfm-import-changed", &view) - .map_err(|error| error.to_string())?; - Ok(view) + let Some(session) = service.snapshot().await else { + let view = service.state().await; + app.emit("lastfm-import-changed", &view) + .map_err(|error| error.to_string())?; + return Ok(view); + }; + let lastfm = Arc::clone(&app.state::().lastfm); + match lastfm + .with_import_owner(&session.lastfm_username, || async { + let view = service.state().await; + app.emit("lastfm-import-changed", &view) + .map_err(|error| error.to_string())?; + Ok(view) + }) + .await? + { + Some(view) => Ok(view), + None => { + service.suspend_for_account_mismatch().await?; + let view = service.state().await; + app.emit("lastfm-import-changed", &view) + .map_err(|error| error.to_string())?; + Ok(view) + } + } } async fn start_import( app: tauri::AppHandle, defaults: Option, ) -> Result { - let (username, spotify_account_id) = connected_accounts(&app).await?; + let username = lastfm_username(&app).await?; + let history_to = crate::history_cutoff_for_import(&app, &username).await?; let state = app.state::(); let service = Arc::clone(&state.lastfm_import); + if let Some(session) = service.snapshot().await { + if session.phase == ImportPhase::Suspended + && requires_spotify_ownership(&session) + && !current_spotify_binding_is_current(&app, service.as_ref(), true).await? + { + let view = service.state().await; + app.emit("lastfm-import-changed", &view) + .map_err(|error| error.to_string())?; + return Ok(view); + } + } let view = service - .start_or_resume(&username, &spotify_account_id, crate::unix_now(), defaults) + .start_or_resume(&username, history_to, defaults) .await?; app.emit("lastfm-import-changed", &view) .map_err(|error| error.to_string())?; if service.claim_runner() { let app = app.clone(); tauri::async_runtime::spawn(async move { - run_import(app, service, username, spotify_account_id).await; + run_import(app, service, username).await; }); } Ok(view) } -async fn run_import( - app: tauri::AppHandle, - service: Arc, - username: String, - spotify_account_id: String, -) { +pub(crate) async fn resume_persisted_import(app: tauri::AppHandle) { + let state = app.state::(); + let service = Arc::clone(&state.lastfm_import); + let Some(session) = service.snapshot().await else { + return; + }; + let Some((username, _history_to)) = startup_resume_plan(Some(&session)) else { + return; + }; + let live_username = if session.phase == ImportPhase::Aggregating { + lastfm_username(&app).await.ok() + } else { + None + }; + if !startup_lastfm_identity_matches(&session, live_username.as_deref()) { + if service.suspend_for_account_mismatch().await.is_ok() { + let view = service.state().await; + let _ = app.emit("lastfm-import-changed", &view); + } + return; + } + if service.claim_runner() { + run_import(app, service, username).await; + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum SourceRunnerStep { + Probe, + Page(u32), + Aggregate, +} + +fn source_runner_step(session: &LastFmImportSessionV2) -> SourceRunnerStep { + if session.total_pages.is_none() { + SourceRunnerStep::Probe + } else if session.next_page == 0 { + SourceRunnerStep::Aggregate + } else { + SourceRunnerStep::Page(session.next_page) + } +} + +fn startup_resume_plan(session: Option<&LastFmImportSessionV2>) -> Option<(String, u64)> { + session + .filter(|session| { + matches!( + session.phase, + ImportPhase::Downloading | ImportPhase::Aggregating + ) + }) + .map(|session| (session.lastfm_username.clone(), session.history_to)) +} + +fn startup_lastfm_identity_matches( + session: &LastFmImportSessionV2, + live_username: Option<&str>, +) -> bool { + session.phase != ImportPhase::Aggregating + || live_username == Some(session.lastfm_username.as_str()) +} + +async fn run_import(app: tauri::AppHandle, service: Arc, username: String) { let result = async { loop { let Some(session) = service.snapshot().await else { @@ -1699,63 +2482,79 @@ async fn run_import( match session.phase { ImportPhase::Downloading => { let lastfm = Arc::clone(&app.state::().lastfm); - let payload = match lastfm - .import_recent_tracks_page( - &username, - session.next_page, - session.snapshot_to, - ) - .await - { - Ok(payload) => payload, - Err(error) => { - if error.account_mismatch { - service.suspend_for_account_mismatch().await?; - return Err(error.message); - } - let attempt = service - .snapshot() - .await - .and_then(|session| session.retryable_error) - .map(|error| error.attempt.saturating_add(1)) - .unwrap_or(1); + match source_runner_step(&session) { + SourceRunnerStep::Probe => { + let payload = fetch_import_page_with_retry( + &lastfm, + &service, + &username, + 1, + session.history_to, + ) + .await?; + let parsed = match parse_recent_tracks_page(&payload) { + Ok(parsed) => parsed, + Err(message) => { + service + .set_retryable_error(Some(RetryableError { + message: message.clone(), + attempt: 0, + retryable: false, + })) + .await?; + return Err(message); + } + }; + let Some(total_pages) = parsed.total_pages else { + let message = + "Last.fm metadata did not include a total page count." + .to_string(); + service + .set_retryable_error(Some(RetryableError { + message: message.clone(), + attempt: 0, + retryable: false, + })) + .await?; + return Err(message); + }; service - .set_retryable_error(RetryableError { - message: error.message.clone(), - attempt: if error.retryable { attempt } else { 0 }, - retryable: error.retryable, - }) + .set_metadata(total_pages, parsed.total.unwrap_or_default()) .await?; - return Err(error.message); } - }; - let parsed = match parse_recent_tracks_page(&payload) { - Ok(parsed) => parsed, - Err(message) => { - let attempt = service - .snapshot() - .await - .and_then(|session| session.retryable_error) - .map(|error| error.attempt.saturating_add(1)) - .unwrap_or(1); - service - .set_retryable_error(RetryableError { - message: message.clone(), - attempt, - retryable: false, - }) - .await?; - return Err(message); + SourceRunnerStep::Aggregate => { + service.aggregate_cached(Some(lastfm.as_ref())).await?; + } + SourceRunnerStep::Page(page) => { + let payload = fetch_import_page_with_retry( + &lastfm, + &service, + &username, + page, + session.history_to, + ) + .await?; + let parsed = match parse_recent_tracks_page(&payload) { + Ok(parsed) => parsed, + Err(message) => { + service + .set_retryable_error(Some(RetryableError { + message: message.clone(), + attempt: 0, + retryable: false, + })) + .await?; + return Err(message); + } + }; + service.checkpoint_page(page, &parsed).await?; } - }; - service.checkpoint_page(session.next_page, &parsed).await?; - let _ = app.emit("lastfm-import-changed", service.state().await); - if parsed.tracks.len() == LASTFM_PAGE_LIMIT as usize { - tokio::time::sleep(Duration::from_millis(250)).await; } + let _ = emit_import_changed(&app, &service).await; } - ImportPhase::Matching => { - run_matching(&app, &service, &username, &spotify_account_id).await?; + ImportPhase::Aggregating => { + let lastfm = Arc::clone(&app.state::().lastfm); + service.aggregate_cached(Some(lastfm.as_ref())).await?; } ImportPhase::Review | ImportPhase::Done | ImportPhase::Suspended => break, } @@ -1764,23 +2563,66 @@ async fn run_import( } .await; if let Err(error) = result { - let already_recorded = service - .snapshot() + let _ = service + .set_retryable_error(Some(RetryableError { + message: error, + attempt: 0, + retryable: false, + })) + .await; + } + service.release_runner(); + let _ = emit_import_changed(&app, &service).await; +} + +async fn fetch_import_page_with_retry( + lastfm: &crate::lastfm::Service, + service: &Service, + username: &str, + page: u32, + history_to: u64, +) -> Result { + loop { + match lastfm + .import_recent_tracks_page(username, page, history_to) .await - .and_then(|session| session.retryable_error) - .is_some(); - if !already_recorded { - let _ = service - .set_retryable_error(RetryableError { - message: error, - attempt: 0, - retryable: true, - }) - .await; + { + Ok(payload) => { + service.set_retryable_error(None).await?; + return Ok(payload); + } + Err(error) if error.account_mismatch => { + service.suspend_for_account_mismatch().await?; + return Err(error.message); + } + Err(error) if error.retryable => { + let attempt = service + .snapshot() + .await + .and_then(|session| session.retryable_error) + .map(|error| error.attempt.saturating_add(1)) + .unwrap_or(1); + service + .set_retryable_error(Some(RetryableError { + message: error.message, + attempt, + retryable: true, + })) + .await?; + tokio::time::sleep(crate::lastfm::import_retry_delay(usize::MAX)).await; + } + Err(error) => { + service + .set_retryable_error(Some(RetryableError { + message: error.message.clone(), + attempt: 0, + retryable: false, + })) + .await?; + return Err(error.message); + } } } - service.release_runner(); - let _ = app.emit("lastfm-import-changed", service.state().await); } fn album_search_term(artist: &str, album: &str) -> String { @@ -1831,172 +2673,222 @@ fn candidate_rank(relation: Option) -> u8 { } } -async fn checkpoint_matching( - app: &tauri::AppHandle, - service: &Service, - username: &str, - spotify_account_id: &str, - results: Vec, -) -> Result<(), String> { - let (current_username, current_account_id) = assert_current_account(app, service).await?; - if current_username != username || current_account_id != spotify_account_id { - return Err("The connected account changed during matching.".into()); +async fn match_batch( + provider: &retune_spotify::client::SpotifyClient, + artist: &str, + album: &str, + rows: &[SourceRow], +) -> Result, String> +where + T: retune_spotify::client::Transport, + S: retune_spotify::tokens::TokenStore, +{ + if album.is_empty() { + let mut matches = Vec::new(); + for row in rows { + let search_term = track_search_term(artist, &row.track); + let results = crate::provider::search_tracks(provider, &search_term).await?; + let mut candidates = results + .items + .into_iter() + .map(|track| AlbumCandidate { + uri: track.uri.clone(), + name: track.name.clone(), + artist: track.artist.clone(), + track_uris: vec![track.uri.clone()], + track_names: vec![track.name.clone()], + track_artists: vec![track.artist], + track_albums: vec![track.alb], + relation: None, + }) + .collect::>(); + classify_album_candidates_by_name(std::slice::from_ref(&row.track), &mut candidates); + matches.push(match_result_for( + row.stable_id.clone(), + search_term, + candidates, + &row.track, + false, + )); + } + return Ok(matches); } - service - .set_matches_during_matching(username, spotify_account_id, results) - .await + let search_term = album_search_term(artist, album); + let source_track_names = rows.iter().map(|row| row.track.clone()).collect::>(); + let candidates = album_candidates(provider, &search_term, &source_track_names).await?; + Ok(rows + .iter() + .map(|row| { + match_result_for( + row.stable_id.clone(), + search_term.clone(), + candidates.clone(), + &row.track, + false, + ) + }) + .collect()) } -async fn run_matching( - app: &tauri::AppHandle, +async fn current_matching_account_locked( + state: &crate::AppState, service: &Service, - username: &str, - spotify_account_id: &str, -) -> Result<(), String> { - let state = app.state::(); - let (current_username, current_account_id) = assert_current_account(app, service).await?; - if current_username != username || current_account_id != spotify_account_id { - return Err("The connected Spotify account changed during matching.".into()); - } +) -> Result<(String, String), String> { + let (username, spotify_account_id) = connected_accounts_locked(state).await?; let Some(session) = service.snapshot().await else { - return Ok(()); + return Err("No Last.fm import session is active.".into()); }; - if session.spotify_account_id != spotify_account_id || session.phase != ImportPhase::Matching { - return Ok(()); + if !session_account_matches(&session, &username, &spotify_account_id, false) { + service.suspend_for_account_mismatch().await?; + return Err( + "The saved Last.fm import belongs to a different account; it is suspended for safety." + .into(), + ); } - let provider = crate::provider_from(&state)?; - let mut groups = BTreeMap::<(String, String), Vec>::new(); - for row in session.rows { - if !session.matches.contains_key(&row.stable_id) { - groups - .entry((row.artist.clone(), row.album.clone())) - .or_default() - .push(row); - } + if !review_phase_allowed(session.phase) { + return Err("Last.fm matching is available only after source review begins.".into()); } - for ((artist, album), rows) in groups { - if album.is_empty() { - for row in rows { - let search_term = track_search_term(&artist, &row.track); - let results = - crate::provider::search_tracks(provider.as_ref(), &search_term).await?; - let mut candidates = results - .items - .into_iter() - .map(|track| AlbumCandidate { - uri: track.uri.clone(), - name: track.name.clone(), - artist: track.artist.clone(), - track_uris: vec![track.uri.clone()], - track_names: vec![track.name.clone()], - track_artists: vec![track.artist], - track_albums: vec![track.alb], - relation: None, - }) - .collect::>(); - classify_album_candidates_by_name( - std::slice::from_ref(&row.track), - &mut candidates, - ); - let selected = candidates - .iter() - .min_by_key(|candidate| candidate_rank(candidate.relation)); - let confidence = selected.map(|candidate| match candidate.relation { - Some(AlbumRelation::BestMatch) => Confidence::Exact, - Some(AlbumRelation::SameSongs | AlbumRelation::Superset) => Confidence::Likely, - None => Confidence::Low, - }); - let selected_uri = selected - .filter(|candidate| candidate.relation.is_some()) - .map(|candidate| candidate.uri.clone()); - let mut track_matches = BTreeMap::new(); - if let Some(uri) = selected_uri.clone() { - track_matches.insert(row.stable_id.clone(), uri); - } - checkpoint_matching( - app, - service, - username, - spotify_account_id, - vec![MatchResult { - source_id: row.stable_id, - search_term, - confidence, - selected_uri, - candidates, - track_matches, - }], - ) - .await?; - let _ = app.emit("lastfm-import-changed", service.state().await); - } - continue; - } - let search_term = album_search_term(&artist, &album); - let results = crate::provider::search_albums(provider.as_ref(), &search_term).await?; - let mut candidates = Vec::new(); - for album_result in results.items.into_iter().take(10) { - let tracks = - crate::provider::album_tracks(provider.as_ref(), &album_result.uri).await?; - candidates.push(AlbumCandidate { - uri: album_result.uri, - name: album_result.name, - artist: album_result.artist, - track_uris: tracks.iter().map(|track| track.uri.clone()).collect(), - track_names: tracks.iter().map(|track| track.name.clone()).collect(), - track_artists: tracks.iter().map(|track| track.art.clone()).collect(), - track_albums: tracks.iter().map(|track| track.alb.clone()).collect(), - relation: None, - }); - } - let source_track_names = rows.iter().map(|row| row.track.clone()).collect::>(); - classify_album_candidates_by_name(&source_track_names, &mut candidates); - let selected = candidates - .iter() - .min_by_key(|candidate| candidate_rank(candidate.relation)); - let confidence = selected.map(|candidate| match candidate.relation { - Some(AlbumRelation::BestMatch) => Confidence::Exact, - Some(AlbumRelation::SameSongs | AlbumRelation::Superset) => Confidence::Likely, - None => Confidence::Low, - }); - let selected_uri = selected - .filter(|candidate| candidate.relation.is_some()) - .map(|candidate| candidate.uri.clone()); - let mut track_matches = BTreeMap::new(); - if let Some(selected) = selected.filter(|candidate| candidate.relation.is_some()) { - for row in &rows { - if let Some(index) = selected - .track_names - .iter() - .position(|name| normalize_for_match(name) == normalize_for_match(&row.track)) - { - if let Some(uri) = selected.track_uris.get(index) { - track_matches.insert(row.stable_id.clone(), uri.clone()); - } - } - } - } - let matches = rows - .into_iter() - .map(|row| MatchResult { - source_id: row.stable_id, - search_term: search_term.clone(), - confidence, - selected_uri: selected_uri.clone(), - candidates: candidates.clone(), - track_matches: track_matches.clone(), + Ok((username, spotify_account_id)) +} + +fn session_account_matches( + session: &LastFmImportSessionV2, + username: &str, + spotify_account_id: &str, + require_spotify_binding: bool, +) -> bool { + session.lastfm_username == username + && session + .spotify_account_id + .as_deref() + .map_or(!require_spotify_binding, |bound| { + bound == spotify_account_id }) - .collect(); - checkpoint_matching(app, service, username, spotify_account_id, matches).await?; - let _ = app.emit("lastfm-import-changed", service.state().await); +} + +async fn cached_spotify_binding_is_current( + app: &tauri::AppHandle, + service: &Service, +) -> Result { + current_spotify_binding_is_current(app, service, false).await +} + +fn cached_spotify_identity_matches( + expected: &str, + library: &crate::store::SpotifyLibraryState, +) -> Option { + library.is_exact().then_some(library.account_id == expected) +} + +async fn lazy_match_page_with_search( + service: &Service, + spotify_library_gate: &tokio::sync::Mutex<()>, + batch_id: u32, + artist: &str, + album: &str, + current_account: A, + search: F, +) -> Result, String> +where + A: Fn() -> AFut, + AFut: Future>, + F: FnOnce(Vec) -> FFut, + FFut: Future, String>>, +{ + let Some(page) = service.page(batch_id, artist, album).await else { + return Ok(None); + }; + let session = service + .snapshot() + .await + .ok_or_else(|| "No Last.fm import session is active.".to_string())?; + if batch_match_plan(&session, Some((batch_id, artist, album))).is_empty() { + let _membership_guard = spotify_library_gate.lock().await; + current_account().await?; + return Ok(Some(page)); + } + + // ponytail: one importer-wide lock; use per-batch locks only if throughput requires it. + let _match_guard = service.lazy_match_lock.lock().await; + let Some(page) = service.page(batch_id, artist, album).await else { + return Ok(None); + }; + let session = service + .snapshot() + .await + .ok_or_else(|| "No Last.fm import session is active.".to_string())?; + if batch_match_plan(&session, Some((batch_id, artist, album))).is_empty() { + let _membership_guard = spotify_library_gate.lock().await; + current_account().await?; + return Ok(Some(page)); } - let (final_username, final_account_id) = assert_current_account(app, service).await?; - if final_username != username || final_account_id != spotify_account_id { - return Err("The connected account changed before matching completed.".into()); + let initial_account = { + let _membership_guard = spotify_library_gate.lock().await; + current_account().await? + }; + let batch = requested_batch(&session, batch_id, artist, album) + .ok_or_else(|| "Unknown Last.fm import review batch.".to_string())?; + let rows_by_id = source_row_map(&session); + let rows = batch_rows(&batch, &rows_by_id) + .into_iter() + .cloned() + .collect::>(); + let results = search(rows).await?; + let _membership_guard = spotify_library_gate.lock().await; + let (username, spotify_account_id) = current_account().await?; + if (username.as_str(), spotify_account_id.as_str()) + != (initial_account.0.as_str(), initial_account.1.as_str()) + { + service.suspend_for_account_mismatch().await?; + return Err( + "The connected Spotify account changed while matching; the import is suspended for safety." + .into(), + ); } service - .finish_matching_if_current(username, spotify_account_id) + .set_matches(&username, &spotify_account_id, batch_id, results) + .await?; + Ok(service.page(batch_id, artist, album).await) +} + +async fn lazy_match_page( + app: &tauri::AppHandle, + service: &Service, + batch_id: u32, + artist: &str, + album: &str, +) -> Result, String> { + let Some(page) = service.page(batch_id, artist, album).await else { + return Ok(None); + }; + let session = service + .snapshot() .await + .ok_or_else(|| "No Last.fm import session is active.".to_string())?; + if batch_match_plan(&session, Some((batch_id, artist, album))).is_empty() { + return cached_spotify_binding_is_current(app, service) + .await + .map(|current| current.then_some(page)); + } + let state = app.state::(); + let state_ref = &*state; + let page = lazy_match_page_with_search( + service, + &state_ref.spotify_library_gate, + batch_id, + artist, + album, + || current_matching_account_locked(state_ref, service), + |rows| async move { + let provider = crate::provider_from(state_ref)?; + match_batch(provider.as_ref(), artist, album, &rows).await + }, + ) + .await?; + if page.is_some() { + let _ = app.emit("lastfm-import-changed", service.state().await); + } + Ok(page) } fn matched_track_uri(result: &MatchResult, source_id: &str) -> Option { @@ -2033,6 +2925,86 @@ fn matched_track_uri_for_row(result: &MatchResult, row: &SourceRow) -> Option, +) -> Vec<(u32, String, String)> { + let rows_by_id = source_row_map(session); + review_batches(session) + .into_iter() + .filter_map(|batch| { + let rows = batch_rows(&batch, &rows_by_id); + let first = rows.first()?; + let selected = requested.is_some_and(|(requested_page, artist, album)| { + requested_page == batch.page && artist == first.artist && album == first.album + }); + let remaining = requested.is_none() + && rows + .iter() + .any(|row| is_actionable(session, &row.stable_id)); + if (selected || remaining) + && rows + .iter() + .any(|row| !session.matches.contains_key(&row.stable_id)) + { + Some((batch.page, first.artist.clone(), first.album.clone())) + } else { + None + } + }) + .collect() +} + +fn accept_all_entity_uris(session: &LastFmImportSessionV2) -> (BTreeSet, BTreeSet) { + let mut album_uris = BTreeSet::new(); + let mut track_uris = BTreeSet::new(); + let rows_by_id = source_row_map(session); + for batch in review_batches(session) { + let rows = batch_rows(&batch, &rows_by_id); + let Some(first) = rows.first() else { + continue; + }; + let options = session.options_for_batch(batch.page, &first.artist, &first.album); + let selected = rows + .into_iter() + .filter(|row| { + options.selected_track_ids.contains(&row.stable_id) + && is_actionable(session, &row.stable_id) + }) + .collect::>(); + if !options.import_content { + continue; + } + if options.whole_album { + for row in selected { + if let Some(uri) = session + .matches + .get(&row.stable_id) + .and_then(|result| { + result.selected_uri.as_deref().or_else(|| { + best_candidate(result).map(|candidate| candidate.uri.as_str()) + }) + }) + .filter(|uri| uri.starts_with("spotify:album:")) + { + album_uris.insert(uri.to_owned()); + } + } + } else { + for row in selected { + if let Some(uri) = session + .matches + .get(&row.stable_id) + .and_then(|result| matched_track_uri_for_row(result, row)) + { + track_uris.insert(uri); + } + } + } + } + (album_uris, track_uris) +} + fn membership_uris_for_import( import_content: bool, whole_album: bool, @@ -2074,7 +3046,7 @@ fn committed_source_ids( } fn historical_count_for_target( - session: &LastFmImportSessionV1, + session: &LastFmImportSessionV2, target_uri: &str, current_rows: &[&SourceRow], current_options: &PageOptions, @@ -2083,6 +3055,7 @@ fn historical_count_for_target( .iter() .map(|row| row.stable_id.as_str()) .collect::>(); + let source_batches = source_batch_map(session); let mut relevant = Vec::new(); for row in &session.rows { let decision = default_decision(session, &row.stable_id); @@ -2091,7 +3064,10 @@ fn historical_count_for_target( let page_options = if current_page { current_options.clone() } else { - session.options_for(&row.artist, &row.album) + source_batches + .get(row.stable_id.as_str()) + .map(|batch_id| session.options_for_batch(*batch_id, &row.artist, &row.album)) + .unwrap_or_else(|| session.options_for(&row.artist, &row.album)) }; if included && page_options.include_historical_play_counts @@ -2148,6 +3124,7 @@ fn update_selected_match( async fn apply_page( app: &tauri::AppHandle, service: &Service, + batch_id: u32, artist: &str, album: &str, selected_ids: &[String], @@ -2160,11 +3137,19 @@ async fn apply_page( let Some(session) = service.snapshot().await else { return Err("No Last.fm import session is active.".into()); }; + let Some(batch) = requested_batch(&session, batch_id, artist, album) else { + return Err("Unknown Last.fm import review batch.".into()); + }; let selected = selected_ids.iter().cloned().collect::>(); - let rows = session - .rows + if selected_ids .iter() - .filter(|row| row.artist == artist && row.album == album) + .any(|id| !batch.source_ids.iter().any(|source_id| source_id == id)) + { + return Err("A selected source row does not belong to this review batch.".into()); + } + let rows_by_id = source_row_map(&session); + let rows = batch_rows(&batch, &rows_by_id) + .into_iter() .filter(|row| selected.contains(&row.stable_id)) .filter(|row| { let decision = default_decision(&session, &row.stable_id); @@ -2277,6 +3262,7 @@ async fn apply_page( .commit_rows( &username, &spotify_account_id, + batch_id, &committed, artist, album, @@ -2425,30 +3411,48 @@ pub(crate) async fn open_lastfm_importer(app: tauri::AppHandle) -> Result<(), St pub(crate) async fn lastfm_import_state(app: tauri::AppHandle) -> Result { let state = app.state::(); let service = &state.lastfm_import; - let identity = if service.snapshot().await.is_none() { - let _membership_guard = state.spotify_library_gate.lock().await; - connected_accounts_locked(&state).await.ok() - } else { + if service.snapshot().await.is_some() { let _ = ensure_import_readable(&app, service.as_ref()).await?; - None - }; - Ok(service.state_with_identity(identity).await) + } + let mut view = service.state().await; + if view.phase.is_none() { + view.username = lastfm_username(&app).await.ok(); + } + Ok(view) } #[tauri::command] pub(crate) async fn lastfm_import_queue( app: tauri::AppHandle, -) -> Result, String> { + cursor: Option, + limit: Option, +) -> Result { let service = &app.state::().lastfm_import; + let cursor = cursor.unwrap_or_default(); + let limit = limit.unwrap_or(LASTFM_QUEUE_PAGE_LIMIT); + if limit == 0 || limit > LASTFM_QUEUE_PAGE_LIMIT { + return Err(format!( + "Last.fm import queue limit must be between 1 and {LASTFM_QUEUE_PAGE_LIMIT}." + )); + } if !ensure_import_readable(&app, service.as_ref()).await? { - return Ok(Vec::new()); + if cursor != 0 { + return Err("Last.fm import queue cursor is out of range.".into()); + } + return Ok(ImportQueuePage { + items: Vec::new(), + cursor, + next_cursor: None, + total: 0, + }); } - Ok(service.queue().await) + service.queue_page(cursor, limit).await } #[tauri::command(rename_all = "camelCase")] pub(crate) async fn lastfm_import_page( app: tauri::AppHandle, + batch_id: u32, artist: String, album: String, ) -> Result, String> { @@ -2456,48 +3460,62 @@ pub(crate) async fn lastfm_import_page( if !ensure_import_readable(&app, service.as_ref()).await? { return Ok(None); } - Ok(service.page(&artist, &album).await) + lazy_match_page(&app, service.as_ref(), batch_id, &artist, &album).await } #[tauri::command(rename_all = "camelCase")] pub(crate) async fn lastfm_import_review( app: tauri::AppHandle, + batch_id: u32, id: String, action: String, artist: String, album: String, ) -> Result { let state = app.state::(); + let _membership_guard = state.spotify_library_gate.lock().await; let (username, spotify_account_id) = - assert_current_account(&app, state.lastfm_import.as_ref()).await?; + assert_current_account_locked(&state, state.lastfm_import.as_ref()).await?; state .lastfm_import .review_action( &username, &spotify_account_id, + batch_id, &id, &action, &artist, &album, ) .await?; + drop(_membership_guard); emit_import_changed(&app, state.lastfm_import.as_ref()).await } #[tauri::command(rename_all = "camelCase")] pub(crate) async fn lastfm_import_options( app: tauri::AppHandle, + batch_id: u32, artist: String, album: String, options: PageOptions, ) -> Result { let state = app.state::(); + let _membership_guard = state.spotify_library_gate.lock().await; let (username, spotify_account_id) = - assert_current_account(&app, state.lastfm_import.as_ref()).await?; + assert_current_account_locked(&state, state.lastfm_import.as_ref()).await?; state .lastfm_import - .update_options(&username, &spotify_account_id, &artist, &album, options) + .update_options( + &username, + &spotify_account_id, + batch_id, + &artist, + &album, + options, + ) .await?; + drop(_membership_guard); emit_import_changed(&app, state.lastfm_import.as_ref()).await } @@ -2508,12 +3526,14 @@ pub(crate) async fn lastfm_import_count_mode( mode: CountMode, ) -> Result { let state = app.state::(); + let _membership_guard = state.spotify_library_gate.lock().await; let (username, spotify_account_id) = - assert_current_account(&app, state.lastfm_import.as_ref()).await?; + assert_current_account_locked(&state, state.lastfm_import.as_ref()).await?; state .lastfm_import .set_count_mode(&username, &spotify_account_id, &target_uri, mode) .await?; + drop(_membership_guard); emit_import_changed(&app, state.lastfm_import.as_ref()).await } @@ -2523,40 +3543,45 @@ pub(crate) async fn lastfm_import_search_terms( show: bool, ) -> Result { let state = app.state::(); + let _membership_guard = state.spotify_library_gate.lock().await; let (username, spotify_account_id) = - assert_current_account(&app, state.lastfm_import.as_ref()).await?; + assert_current_account_locked(&state, state.lastfm_import.as_ref()).await?; state .lastfm_import .set_search_terms(&username, &spotify_account_id, show) .await?; + drop(_membership_guard); emit_import_changed(&app, state.lastfm_import.as_ref()).await } #[tauri::command(rename_all = "camelCase")] pub(crate) async fn lastfm_import_select_match( app: tauri::AppHandle, + batch_id: u32, id: String, uri: String, ) -> Result { let state = app.state::(); + let _membership_guard = state.spotify_library_gate.lock().await; let (username, spotify_account_id) = - assert_current_account(&app, state.lastfm_import.as_ref()).await?; + assert_current_account_locked(&state, state.lastfm_import.as_ref()).await?; state .lastfm_import - .select_match(&username, &spotify_account_id, &id, &uri) + .select_match(&username, &spotify_account_id, batch_id, &id, &uri) .await?; + drop(_membership_guard); emit_import_changed(&app, state.lastfm_import.as_ref()).await } #[tauri::command(rename_all = "camelCase")] pub(crate) async fn lastfm_import_change_track( app: tauri::AppHandle, + batch_id: u32, id: String, query: String, ) -> Result { let state = app.state::(); - let (username, spotify_account_id) = - assert_current_account(&app, state.lastfm_import.as_ref()).await?; + let _ = assert_current_account(&app, state.lastfm_import.as_ref()).await?; let session = state .lastfm_import .snapshot() @@ -2567,6 +3592,9 @@ pub(crate) async fn lastfm_import_change_track( .iter() .find(|row| row.stable_id == id) .ok_or_else(|| "Unknown Last.fm import source row.".to_string())?; + if requested_batch(&session, batch_id, &row.artist, &row.album).is_none() { + return Err("The source row does not belong to this review batch.".into()); + } let search_term = if query.trim().is_empty() { track_search_term(&row.artist, &row.track) } else { @@ -2595,23 +3623,26 @@ pub(crate) async fn lastfm_import_change_track( session.matches.get(&id), &id, ); - let _ = assert_current_account(&app, state.lastfm_import.as_ref()).await?; + let _membership_guard = state.spotify_library_gate.lock().await; + let (username, spotify_account_id) = + assert_current_account_locked(&state, state.lastfm_import.as_ref()).await?; state .lastfm_import - .set_match(&username, &spotify_account_id, result) + .set_match(&username, &spotify_account_id, batch_id, result) .await?; + drop(_membership_guard); emit_import_changed(&app, state.lastfm_import.as_ref()).await } #[tauri::command(rename_all = "camelCase")] pub(crate) async fn lastfm_import_change_album( app: tauri::AppHandle, + batch_id: u32, id: String, query: String, ) -> Result { let state = app.state::(); - let (username, spotify_account_id) = - assert_current_account(&app, state.lastfm_import.as_ref()).await?; + let _ = assert_current_account(&app, state.lastfm_import.as_ref()).await?; let session = state .lastfm_import .snapshot() @@ -2622,10 +3653,11 @@ pub(crate) async fn lastfm_import_change_album( .iter() .find(|row| row.stable_id == id) .ok_or_else(|| "Unknown Last.fm import source row.".to_string())?; - let related = session - .rows - .iter() - .filter(|candidate| candidate.artist == row.artist && candidate.album == row.album) + let batch = requested_batch(&session, batch_id, &row.artist, &row.album) + .ok_or_else(|| "The source row does not belong to this review batch.".to_string())?; + let rows_by_id = source_row_map(&session); + let related = batch_rows(&batch, &rows_by_id) + .into_iter() .map(|candidate| candidate.track.clone()) .collect::>(); let search_term = if query.trim().is_empty() { @@ -2635,10 +3667,8 @@ pub(crate) async fn lastfm_import_change_album( }; let provider = crate::provider_from(&state)?; let candidates = album_candidates(provider.as_ref(), &search_term, &related).await?; - let matches = session - .rows - .iter() - .filter(|candidate| candidate.artist == row.artist && candidate.album == row.album) + let matches = batch_rows(&batch, &rows_by_id) + .into_iter() .map(|candidate_row| { preserve_match_selection( match_result_for( @@ -2653,17 +3683,21 @@ pub(crate) async fn lastfm_import_change_album( ) }) .collect(); - let _ = assert_current_account(&app, state.lastfm_import.as_ref()).await?; + let _membership_guard = state.spotify_library_gate.lock().await; + let (username, spotify_account_id) = + assert_current_account_locked(&state, state.lastfm_import.as_ref()).await?; state .lastfm_import - .set_matches(&username, &spotify_account_id, matches) + .set_matches(&username, &spotify_account_id, batch_id, matches) .await?; + drop(_membership_guard); emit_import_changed(&app, state.lastfm_import.as_ref()).await } #[tauri::command(rename_all = "camelCase")] pub(crate) async fn lastfm_import_apply( app: tauri::AppHandle, + batch_id: u32, artist: String, album: String, selected_ids: Vec, @@ -2673,6 +3707,7 @@ pub(crate) async fn lastfm_import_apply( let view = apply_page( &app, state.lastfm_import.as_ref(), + batch_id, &artist, &album, &selected_ids, @@ -2684,18 +3719,73 @@ pub(crate) async fn lastfm_import_apply( Ok(view) } +#[tauri::command] +pub(crate) async fn lastfm_import_prepare_accept_all( + app: tauri::AppHandle, +) -> Result { + let state = app.state::(); + let service = state.lastfm_import.as_ref(); + if !ensure_import_readable(&app, service).await? { + return Ok(AcceptAllSummary { + album_entities: 0, + track_entities: 0, + }); + } + let app_for_prepare = app.clone(); + prepare_accept_all_batches(service, |batch_id, artist, album| { + let app = app_for_prepare.clone(); + async move { + lazy_match_page(&app, service, batch_id, &artist, &album) + .await + .map(|_| ()) + } + }) + .await +} + +async fn prepare_accept_all_batches( + service: &Service, + mut prepare: F, +) -> Result +where + F: FnMut(u32, String, String) -> Fut, + Fut: Future>, +{ + let session = service + .snapshot() + .await + .ok_or_else(|| "No Last.fm import session is active.".to_string())?; + for (batch_id, artist, album) in batch_match_plan(&session, None) { + prepare(batch_id, artist, album).await?; + } + let session = service + .snapshot() + .await + .ok_or_else(|| "No Last.fm import session is active.".to_string())?; + let (albums, tracks) = accept_all_entity_uris(&session); + Ok(AcceptAllSummary { + album_entities: albums.len() as u32, + track_entities: tracks.len() as u32, + }) +} + #[tauri::command(rename_all = "camelCase")] pub(crate) async fn lastfm_import_accept_all_page( app: tauri::AppHandle, + batch_id: u32, artist: String, album: String, ) -> Result { let state = app.state::(); + let _membership_guard = state.spotify_library_gate.lock().await; let (username, spotify_account_id) = - assert_current_account(&app, state.lastfm_import.as_ref()).await?; - let Some(page) = state.lastfm_import.page(&artist, &album).await else { + assert_current_account_locked(&state, state.lastfm_import.as_ref()).await?; + let Some(page) = state.lastfm_import.page(batch_id, &artist, &album).await else { return Ok(state.lastfm_import.state().await); }; + if page.rows.iter().any(|item| item.match_result.is_none()) { + return Err("Prepare Accept All before applying its confirmed batches.".into()); + } let mut selected_album_uris = BTreeSet::new(); for item in &page.rows { if !page @@ -2726,6 +3816,7 @@ pub(crate) async fn lastfm_import_accept_all_page( .select_match( &username, &spotify_account_id, + batch_id, &item.source.stable_id, &candidate.uri, ) @@ -2737,13 +3828,15 @@ pub(crate) async fn lastfm_import_accept_all_page( .select_match( &username, &spotify_account_id, + batch_id, &item.source.stable_id, &candidate.uri, ) .await?; } } - let Some(page) = state.lastfm_import.page(&artist, &album).await else { + drop(_membership_guard); + let Some(page) = state.lastfm_import.page(batch_id, &artist, &album).await else { return Ok(state.lastfm_import.state().await); }; let selected_ids = page @@ -2765,6 +3858,7 @@ pub(crate) async fn lastfm_import_accept_all_page( let view = apply_page( &app, state.lastfm_import.as_ref(), + batch_id, &artist, &album, &selected_ids, @@ -2784,11 +3878,11 @@ pub(crate) async fn start_lastfm_import( start_import(app, defaults).await } -pub(crate) fn default_decision(session: &LastFmImportSessionV1, id: &str) -> RowDecision { +pub(crate) fn default_decision(session: &LastFmImportSessionV2, id: &str) -> RowDecision { session.decisions.get(id).cloned().unwrap_or_default() } -fn locked_count_modes(session: &LastFmImportSessionV1) -> BTreeSet { +fn locked_count_modes(session: &LastFmImportSessionV2) -> BTreeSet { session .rows .iter() @@ -2802,10 +3896,73 @@ fn locked_count_modes(session: &LastFmImportSessionV1) -> BTreeSet { .collect() } -fn queue_status(session: &LastFmImportSessionV1, rows: &[&SourceRow]) -> Option { - if rows +fn queue_item( + session: &LastFmImportSessionV2, + batch: &ImportBatch, + rows: &[&SourceRow], +) -> Option { + let first = rows.first()?; + let artist = first.artist.clone(); + let album = first.album.clone(); + let options = session.options_for_batch(batch.page, &artist, &album); + let remaining = rows.iter().any(|row| { + let decision = default_decision(session, &row.stable_id); + matches!(decision.status, RowStatus::Pending | RowStatus::Skipped) && !decision.excluded + }); + let selected = rows .iter() - .all(|row| default_decision(session, &row.stable_id).excluded) + .filter(|row| { + let decision = default_decision(session, &row.stable_id); + options.selected_track_ids.contains(&row.stable_id) + && matches!(decision.status, RowStatus::Pending | RowStatus::Skipped) + && !decision.excluded + }) + .collect::>(); + let mut album_entities = 0; + let mut track_uris = BTreeSet::new(); + if options.import_content { + if options.whole_album { + album_entities = selected + .iter() + .filter_map(|row| session.matches.get(&row.stable_id)) + .filter_map(|result| { + result + .selected_uri + .as_deref() + .or_else(|| best_candidate(result).map(|candidate| candidate.uri.as_str())) + }) + .any(|uri| uri.starts_with("spotify:album:")) as u32; + } else { + for row in &selected { + if let Some(result) = session.matches.get(&row.stable_id) { + if let Some(uri) = matched_track_uri_for_row(result, row) { + track_uris.insert(uri); + } + } + } + } + } + Some(ImportQueueItem { + page: batch.page, + artist, + album, + play_count: rows + .iter() + .map(|row| row.play_count) + .fold(0, u64::saturating_add), + latest: rows.iter().map(|row| row.latest).max().unwrap_or_default(), + source_count: batch.source_ids.len(), + remaining, + album_entities, + track_entities: track_uris.len() as u32, + status: queue_status(session, rows), + }) +} + +fn queue_status(session: &LastFmImportSessionV2, rows: &[&SourceRow]) -> Option { + if rows + .iter() + .all(|row| default_decision(session, &row.stable_id).excluded) { return Some(QueueStatus::Excluded); } @@ -2828,7 +3985,7 @@ fn queue_status(session: &LastFmImportSessionV1, rows: &[&SourceRow]) -> Option< }) } -fn update_review_phase(session: &mut LastFmImportSessionV1) { +fn update_review_phase(session: &mut LastFmImportSessionV2) { if session.remaining() == 0 { session.phase = ImportPhase::Done; } else if session.phase == ImportPhase::Done { @@ -2840,45 +3997,50 @@ fn review_phase_allowed(phase: ImportPhase) -> bool { matches!(phase, ImportPhase::Review | ImportPhase::Done) } -fn exclude_row(session: &mut LastFmImportSessionV1, id: &str, excluded: bool) { +fn exclude_row(session: &mut LastFmImportSessionV2, id: &str, excluded: bool) { if is_reviewable(session, id) { let decision = session.decisions.entry(id.to_owned()).or_default(); decision.excluded = excluded; } } -fn is_reviewable(session: &LastFmImportSessionV1, id: &str) -> bool { +fn is_reviewable(session: &LastFmImportSessionV2, id: &str) -> bool { matches!( default_decision(session, id).status, RowStatus::Pending | RowStatus::Skipped ) } -fn is_actionable(session: &LastFmImportSessionV1, id: &str) -> bool { +fn is_actionable(session: &LastFmImportSessionV2, id: &str) -> bool { is_reviewable(session, id) && !default_decision(session, id).excluded } -pub(crate) fn ignore_album(session: &mut LastFmImportSessionV1, artist: &str, album: &str) { - let ids = session +fn album_source_ids(session: &LastFmImportSessionV2, artist: &str, album: &str) -> Vec { + session .rows .iter() - .filter(|row| { - row.artist == artist && row.album == album && is_actionable(session, &row.stable_id) - }) + .filter(|row| row.artist == artist && row.album == album) .map(|row| row.stable_id.clone()) - .collect::>(); + .collect() +} + +#[cfg(test)] +pub(crate) fn ignore_album(session: &mut LastFmImportSessionV2, artist: &str, album: &str) { + let ids = album_source_ids(session, artist, album); for id in ids { - session.decisions.insert( - id, - RowDecision { - status: RowStatus::IgnoredAlbum, - excluded: false, - }, - ); + if is_actionable(session, &id) { + session.decisions.insert( + id, + RowDecision { + status: RowStatus::IgnoredAlbum, + excluded: false, + }, + ); + } } } -pub(crate) fn ignore_artist(session: &mut LastFmImportSessionV1, artist: &str) { +pub(crate) fn ignore_artist(session: &mut LastFmImportSessionV2, artist: &str) { let ids = session .rows .iter() @@ -2896,51 +4058,1168 @@ pub(crate) fn ignore_artist(session: &mut LastFmImportSessionV1, artist: &str) { } } -pub(crate) fn skip_album(session: &mut LastFmImportSessionV1, artist: &str, album: &str) { - let ids = session - .rows - .iter() - .filter(|row| { - row.artist == artist && row.album == album && is_actionable(session, &row.stable_id) - }) - .map(|row| row.stable_id.clone()) - .collect::>(); +#[cfg(test)] +pub(crate) fn skip_album(session: &mut LastFmImportSessionV2, artist: &str, album: &str) { + let ids = album_source_ids(session, artist, album); for id in ids { - session.decisions.insert( - id, - RowDecision { - status: RowStatus::Skipped, - excluded: false, + if is_actionable(session, &id) { + session.decisions.insert( + id, + RowDecision { + status: RowStatus::Skipped, + excluded: false, + }, + ); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::store::{SavedAlbumRecord, SpotifyLibraryState}; + use retune_core::model::SourceId; + #[cfg(unix)] + use std::os::unix::fs::PermissionsExt; + use std::{fs, time::Duration}; + + fn response(entries: Value) -> Value { + serde_json::json!({ + "recenttracks": { + "track": entries, + "@attr": {"page": "2", "totalPages": "4", "total": "601"} + } + }) + } + + fn scrobble(artist: &str, album: &str, track: &str, timestamp: u64) -> ParsedScrobble { + ParsedScrobble { + artist: artist.into(), + album: album.into(), + track: track.into(), + timestamp, + } + } + + #[test] + fn aggregation_input_is_sorted_oldest_first_with_deterministic_ties() { + let mut scrobbles = vec![ + scrobble("B", "Album", "Track", 20), + scrobble("A", "Album", "Track", 10), + scrobble("A", "Album", "Other", 10), + ]; + sort_scrobbles(&mut scrobbles); + assert_eq!( + scrobbles + .iter() + .map(|row| (row.artist.clone(), row.track.clone(), row.timestamp)) + .collect::>(), + vec![ + ("A".to_owned(), "Other".to_owned(), 10), + ("A".to_owned(), "Track".to_owned(), 10), + ("B".to_owned(), "Track".to_owned(), 20), + ] + ); + } + + #[test] + fn aggregation_handles_a_large_unique_input_with_indexed_rows() { + const UNIQUE_SCROBBLES: usize = 50_000; + let scrobbles = (0..UNIQUE_SCROBBLES) + .map(|index| scrobble("Artist", "Album", &format!("Track {index}"), index as u64)) + .collect::>(); + let mut rows = Vec::new(); + + aggregate_scrobbles(&mut rows, &scrobbles); + + assert_eq!(rows.len(), UNIQUE_SCROBBLES); + assert_eq!(rows[0].track, "Track 0"); + assert_eq!(rows[UNIQUE_SCROBBLES - 1].track, "Track 49999"); + } + + #[test] + fn source_runner_plans_probe_descending_pages_and_aggregate_without_cursor_advance() { + let mut session = + LastFmImportSessionV2::new_with_defaults("user".into(), 100, ImportDefaults::default()); + assert_eq!(source_runner_step(&session), SourceRunnerStep::Probe); + + session.total_pages = Some(3); + session.next_page = 3; + session.retryable_error = Some(RetryableError { + message: "temporary".into(), + attempt: 1, + retryable: true, + }); + assert_eq!(source_runner_step(&session), SourceRunnerStep::Page(3)); + assert_eq!(session.next_page, 3); + assert_eq!(session.downloaded_pages, 0); + + session.downloaded_pages = 1; + session.next_page = 2; + assert_eq!(source_runner_step(&session), SourceRunnerStep::Page(2)); + session.downloaded_pages = 2; + session.next_page = 1; + assert_eq!(source_runner_step(&session), SourceRunnerStep::Page(1)); + session.downloaded_pages = 3; + session.next_page = 0; + assert_eq!(source_runner_step(&session), SourceRunnerStep::Aggregate); + } + + #[test] + fn startup_resume_plan_uses_the_persisted_source_identity_only() { + let mut session = LastFmImportSessionV2::new_with_defaults( + "fixed-user".into(), + 1786804381, + ImportDefaults::default(), + ); + assert_eq!( + startup_resume_plan(Some(&session)), + Some(("fixed-user".into(), 1786804381)) + ); + assert!(startup_lastfm_identity_matches( + &session, + Some("other-user") + )); + session.phase = ImportPhase::Aggregating; + assert_eq!( + startup_resume_plan(Some(&session)), + Some(("fixed-user".into(), 1786804381)) + ); + assert!(startup_lastfm_identity_matches( + &session, + Some("fixed-user") + )); + assert!(!startup_lastfm_identity_matches( + &session, + Some("other-user") + )); + assert!(!startup_lastfm_identity_matches(&session, None)); + session.phase = ImportPhase::Review; + assert_eq!(startup_resume_plan(Some(&session)), None); + assert_eq!(startup_resume_plan(None), None); + } + + #[test] + fn cached_spotify_identity_only_trusts_an_exact_matching_cache() { + let mut library = SpotifyLibraryState { + account_id: "spotify-a".into(), + complete: true, + ..SpotifyLibraryState::default() + }; + assert_eq!( + cached_spotify_identity_matches("spotify-a", &library), + Some(true) + ); + assert_eq!( + cached_spotify_identity_matches("spotify-b", &library), + Some(false) + ); + + library.complete = false; + assert_eq!(cached_spotify_identity_matches("spotify-a", &library), None); + } + + #[test] + fn session_account_matching_requires_bound_identity_for_owned_mutations() { + let mut session = LastFmImportSessionV2::new("lastfm-user".into(), "spotify-a".into(), 1); + assert!(session_account_matches( + &session, + "lastfm-user", + "spotify-a", + true + )); + assert!(!session_account_matches( + &session, + "lastfm-user", + "spotify-b", + true + )); + + session.spotify_account_id = None; + assert!(!session_account_matches( + &session, + "lastfm-user", + "spotify-a", + true + )); + assert!(session_account_matches( + &session, + "lastfm-user", + "spotify-a", + false + )); + } + + #[test] + fn source_and_review_phases_choose_the_correct_account_boundary() { + let mut session = LastFmImportSessionV2::new_with_defaults( + "lastfm-user".into(), + 1, + ImportDefaults::default(), + ); + assert!(!requires_spotify_ownership(&session)); + session.total_pages = Some(1); + session.phase = ImportPhase::Aggregating; + assert!(!requires_spotify_ownership(&session)); + + session.phase = ImportPhase::Review; + assert!(!requires_spotify_ownership(&session)); + session.spotify_account_id = Some("spotify-a".into()); + assert!(requires_spotify_ownership(&session)); + session.phase = ImportPhase::Done; + assert!(requires_spotify_ownership(&session)); + session.phase = ImportPhase::Suspended; + assert!(requires_spotify_ownership(&session)); + session.spotify_account_id = None; + assert!(!requires_spotify_ownership(&session)); + } + + fn parsed_page( + page: u32, + total_pages: u32, + tracks: Vec, + ) -> ParsedRecentTracksPage { + ParsedRecentTracksPage { + page, + total_pages: Some(total_pages), + total: Some(tracks.len() as u64), + tracks, + ..ParsedRecentTracksPage::default() + } + } + + async fn start_bound(service: &Service, username: &str, spotify: &str, history_to: u64) { + service + .start_or_resume(username, history_to, None) + .await + .unwrap(); + let mut session = service.snapshot().await.unwrap(); + session.spotify_account_id = Some(spotify.into()); + service.save(session).await.unwrap(); + } + + #[test] + fn manifest_is_authoritative_and_acknowledged_page_damage_quarantines_snapshot() { + let dir = tempfile::tempdir().unwrap(); + let store = ImportSessionStore::new(dir.path()); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 42); + session.total_pages = Some(2); + session.next_page = 2; + let orphan = store.page_path(&session.cache_id, 2); + fs::create_dir_all(orphan.parent().unwrap()).unwrap(); + fs::write(&orphan, b"orphan").unwrap(); + assert!(store.validate_cache(&session).is_ok()); + + store + .write_page( + &session, + &parsed_page(2, 2, vec![scrobble("Artist", "Album", "Track", 2)]), + ) + .unwrap(); + assert!(store.validate_cache(&session).is_ok()); + fs::remove_file(&orphan).unwrap(); + session.downloaded_pages = 1; + session.next_page = 1; + store.save(&session).unwrap(); + assert!(store.load().unwrap().is_none()); + assert!(fs::read_dir(dir.path()).unwrap().any(|entry| { + entry + .unwrap() + .file_name() + .to_string_lossy() + .contains("quarantine") + })); + + let dir = tempfile::tempdir().unwrap(); + let store = ImportSessionStore::new(dir.path()); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 42); + session.total_pages = Some(1); + session.next_page = 0; + store + .write_page( + &session, + &parsed_page(1, 1, vec![scrobble("Artist", "Album", "Track", 2)]), + ) + .unwrap(); + session.downloaded_pages = 1; + store.save(&session).unwrap(); + let page_path = store.page_path(&session.cache_id, 1); + let damaged = CachedRawPage { + lastfm_username: session.lastfm_username.clone(), + history_to: 43, + total_pages: 1, + parsed: parsed_page(1, 1, vec![scrobble("Artist", "Album", "Track", 2)]), + }; + fs::write(&page_path, serde_json::to_vec(&damaged).unwrap()).unwrap(); + assert!(store.load().unwrap().is_none()); + + let dir = tempfile::tempdir().unwrap(); + let store = ImportSessionStore::new(dir.path()); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 42); + session.total_pages = Some(1); + session.next_page = 0; + session.downloaded_pages = 1; + let manifest = RawCacheManifest { + version: SESSION_VERSION, + cache_id: session.cache_id.clone(), + lastfm_username: session.lastfm_username.clone(), + history_to: session.history_to, + total_pages: 1, + pages: BTreeMap::from([(1, MAX_RAW_CACHE_BYTES + 1)]), + }; + fs::create_dir_all(store.cache_path(&session.cache_id)).unwrap(); + fs::write( + store.manifest_path(&session.cache_id), + serde_json::to_vec(&manifest).unwrap(), + ) + .unwrap(); + store.save(&session).unwrap(); + assert!(store.load().unwrap().is_none()); + } + + #[test] + fn cache_validation_rejects_skipped_acknowledged_pages_and_malformed_cursors() { + let dir = tempfile::tempdir().unwrap(); + let store = ImportSessionStore::new(dir.path()); + let mut skipped = LastFmImportSessionV2::new("user".into(), "spotify".into(), 42); + skipped.total_pages = Some(3); + skipped.downloaded_pages = 2; + skipped.next_page = 1; + for page in [3, 1] { + store + .write_page( + &skipped, + &parsed_page( + page, + 3, + vec![scrobble("Artist", "Album", "Track", page as u64)], + ), + ) + .unwrap(); + } + assert!(store.validate_cache(&skipped).is_err()); + store.save(&skipped).unwrap(); + assert!(store.load().unwrap().is_none()); + + let dir = tempfile::tempdir().unwrap(); + let store = ImportSessionStore::new(dir.path()); + let mut malformed = LastFmImportSessionV2::new("user".into(), "spotify".into(), 42); + malformed.total_pages = Some(3); + malformed.downloaded_pages = 2; + malformed.next_page = 0; + for page in [3, 2] { + store + .write_page( + &malformed, + &parsed_page( + page, + 3, + vec![scrobble("Artist", "Album", "Track", page as u64)], + ), + ) + .unwrap(); + } + assert!(store.validate_cache(&malformed).is_err()); + store.save(&malformed).unwrap(); + assert!(store.load().unwrap().is_none()); + } + + #[tokio::test] + async fn suspended_completed_source_revalidates_cache_before_aggregation() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + service.start_or_resume("user", 500, None).await.unwrap(); + service.set_metadata(2, 2).await.unwrap(); + for page in [2, 1] { + service + .checkpoint_page( + page, + &parsed_page( + page, + 2, + vec![scrobble("Artist", "Album", "Track", page as u64)], + ), + ) + .await + .unwrap(); + } + assert_eq!( + service.snapshot().await.unwrap().phase, + ImportPhase::Aggregating + ); + service.suspend_for_account_mismatch().await.unwrap(); + let suspended = service.snapshot().await.unwrap(); + let store = ImportSessionStore::new(dir.path()); + fs::remove_file(store.page_path(&suspended.cache_id, 1)).unwrap(); + + let reloaded = Service::new(dir.path()); + assert!(reloaded.snapshot().await.is_none()); + reloaded.start_or_resume("user", 500, None).await.unwrap(); + let fresh = reloaded.snapshot().await.unwrap(); + assert_eq!(fresh.phase, ImportPhase::Downloading); + assert_eq!(fresh.downloaded_pages, 0); + assert_eq!(fresh.next_page, 1); + } + + #[tokio::test] + async fn suspended_source_revalidates_cache_but_review_survives_deleted_raw_cache() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + service.start_or_resume("user", 500, None).await.unwrap(); + service.set_metadata(2, 2).await.unwrap(); + service + .checkpoint_page( + 2, + &parsed_page(2, 2, vec![scrobble("Artist", "Album", "Track", 2)]), + ) + .await + .unwrap(); + service.suspend_for_account_mismatch().await.unwrap(); + let suspended = service.snapshot().await.unwrap(); + let store = ImportSessionStore::new(dir.path()); + fs::remove_file(store.page_path(&suspended.cache_id, 2)).unwrap(); + + service.start_or_resume("user", 500, None).await.unwrap(); + let restarted = service.snapshot().await.unwrap(); + assert_eq!(restarted.phase, ImportPhase::Downloading); + assert_eq!(restarted.downloaded_pages, 0); + assert_eq!(restarted.next_page, 1); + + let review_dir = tempfile::tempdir().unwrap(); + let review_service = Service::new(review_dir.path()); + review_service + .start_or_resume("user", 500, None) + .await + .unwrap(); + review_service.set_metadata(1, 1).await.unwrap(); + review_service + .checkpoint_page( + 1, + &parsed_page(1, 1, vec![scrobble("Artist", "Album", "Track", 2)]), + ) + .await + .unwrap(); + review_service.aggregate_cached(None).await.unwrap(); + review_service.suspend_for_account_mismatch().await.unwrap(); + assert_eq!( + Service::new(review_dir.path()) + .snapshot() + .await + .unwrap() + .phase, + ImportPhase::Suspended + ); + } + + #[tokio::test] + async fn retry_state_round_trips_without_advancing_the_cursor() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + service + .start_or_resume("lastfm-user", 500, None) + .await + .unwrap(); + service.set_metadata(3, 600).await.unwrap(); + service + .set_retryable_error(Some(RetryableError { + message: "temporary".into(), + attempt: 4, + retryable: true, + })) + .await + .unwrap(); + + let session = Service::new(dir.path()).snapshot().await.unwrap(); + assert_eq!(session.next_page, 3); + assert_eq!(session.downloaded_pages, 0); + assert_eq!(session.retryable_error.unwrap().attempt, 4); + } + + #[tokio::test] + async fn spotify_binding_waits_for_the_first_review_match() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + service + .start_or_resume("lastfm-user", 500, None) + .await + .unwrap(); + assert_eq!(service.snapshot().await.unwrap().spotify_account_id, None); + service.set_metadata(1, 1).await.unwrap(); + service + .checkpoint_page( + 1, + &parsed_page(1, 1, vec![scrobble("Artist", "Album", "Track", 10)]), + ) + .await + .unwrap(); + service.aggregate_cached(None).await.unwrap(); + let source_id = service.snapshot().await.unwrap().rows[0].stable_id.clone(); + service + .set_match( + "lastfm-user", + "spotify-user", + 1, + MatchResult { + source_id, + search_term: "track search".into(), + confidence: Some(Confidence::Exact), + selected_uri: Some("spotify:track:target".into()), + candidates: Vec::new(), + track_matches: BTreeMap::new(), + }, + ) + .await + .unwrap(); + assert_eq!( + service + .snapshot() + .await + .unwrap() + .spotify_account_id + .as_deref(), + Some("spotify-user") + ); + } + + #[tokio::test] + async fn all_pages_are_present_before_aggregation_and_review() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + service + .start_or_resume("lastfm-user", 500, None) + .await + .unwrap(); + service.set_metadata(2, 2).await.unwrap(); + service + .checkpoint_page( + 2, + &parsed_page(2, 2, vec![scrobble("Artist", "Album", "New", 20)]), + ) + .await + .unwrap(); + let partial = service.snapshot().await.unwrap(); + assert_eq!(partial.phase, ImportPhase::Downloading); + assert!(partial.rows.is_empty()); + service + .checkpoint_page( + 1, + &parsed_page(1, 2, vec![scrobble("Artist", "Album", "Old", 10)]), + ) + .await + .unwrap(); + let complete = service.snapshot().await.unwrap(); + assert_eq!(complete.phase, ImportPhase::Aggregating); + assert_eq!(complete.downloaded_pages, 2); + assert!(complete.rows.is_empty()); + service.aggregate_cached(None).await.unwrap(); + let review = service.snapshot().await.unwrap(); + assert_eq!(review.phase, ImportPhase::Review); + assert_eq!( + review + .rows + .iter() + .map(|row| row.track.as_str()) + .collect::>(), + ["Old", "New"] + ); + } + + #[tokio::test] + async fn empty_aggregate_enters_done() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + service + .start_or_resume("lastfm-user", 500, None) + .await + .unwrap(); + service.set_metadata(1, 0).await.unwrap(); + service + .checkpoint_page(1, &parsed_page(1, 1, Vec::new())) + .await + .unwrap(); + + service.aggregate_cached(None).await.unwrap(); + + let session = service.snapshot().await.unwrap(); + assert_eq!(session.phase, ImportPhase::Done); + assert!(session.rows.is_empty()); + assert_eq!(session.remaining(), 0); + } + + #[tokio::test] + async fn checkpoint_discards_rows_at_or_after_history_cutoff() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + service + .start_or_resume("lastfm-user", 500, None) + .await + .unwrap(); + service.set_metadata(1, 3).await.unwrap(); + service + .checkpoint_page( + 1, + &parsed_page( + 1, + 1, + vec![ + scrobble("Artist", "Album", "Before", 499), + scrobble("Artist", "Album", "At cutoff", 500), + scrobble("Artist", "Album", "After", 501), + ], + ), + ) + .await + .unwrap(); + + let session = service.snapshot().await.unwrap(); + assert_eq!(session.included_scrobbles, 1); + assert_eq!(session.phase, ImportPhase::Aggregating); + service.aggregate_cached(None).await.unwrap(); + let session = service.snapshot().await.unwrap(); + assert_eq!(session.rows.len(), 1); + assert_eq!(session.rows[0].track, "Before"); + } + + #[test] + fn cache_identity_uses_exact_username_and_rejects_metadata_mismatch() { + assert_ne!( + snapshot_cache_id("user.name", 42), + snapshot_cache_id("username", 42) + ); + + let dir = tempfile::tempdir().unwrap(); + let store = ImportSessionStore::new(dir.path()); + let mut session = LastFmImportSessionV2::new("user.name".into(), "spotify".into(), 42); + session.total_pages = Some(1); + session.next_page = 1; + store + .write_page( + &session, + &parsed_page(1, 1, vec![scrobble("Artist", "Album", "Track", 2)]), + ) + .unwrap(); + + let mut mismatch = session.clone(); + mismatch.lastfm_username = "user-name".into(); + mismatch.cache_id = session.cache_id.clone(); + assert!(store.validate_cache(&mismatch).is_err()); + store.save(&mismatch).unwrap(); + assert!(store.load().unwrap().is_none()); + } + + #[test] + fn visible_batch_matching_is_lazy_and_accept_all_is_the_bulk_plan() { + let mut session = + LastFmImportSessionV2::new_with_defaults("user".into(), 100, ImportDefaults::default()); + aggregate_scrobbles( + &mut session.rows, + &[ + scrobble("Artist A", "Album A", "One", 1), + scrobble("Artist B", "Album B", "Two", 2), + scrobble("Artist C", "Album C", "Three", 3), + ], + ); + let first_id = session.rows[0].stable_id.clone(); + let first_key = "Artist A\u{1f}Album A".to_owned(); + session.page_options.insert( + first_key, + PageOptions { + selected_track_ids: BTreeSet::from([first_id.clone()]), + ..PageOptions::default() + }, + ); + + assert_eq!( + batch_match_plan(&session, Some((1, "Artist A", "Album A"))), + vec![(1, "Artist A".into(), "Album A".into())] + ); + session.matches.insert( + first_id.clone(), + MatchResult { + source_id: first_id, + search_term: "track".into(), + confidence: Some(Confidence::Exact), + selected_uri: Some("spotify:track:first".into()), + candidates: Vec::new(), + track_matches: BTreeMap::new(), + }, + ); + assert!(batch_match_plan(&session, Some((1, "Artist A", "Album A"))).is_empty()); + assert_eq!( + batch_match_plan(&session, None), + vec![ + (2, "Artist B".into(), "Album B".into()), + (3, "Artist C".into(), "Album C".into()), + ] + ); + } + + #[test] + fn review_batches_split_large_single_groups_into_stable_bounded_pages() { + let rows = (0..205) + .map(|index| SourceRow { + stable_id: format!("source-{index}"), + artist: "Artist".into(), + album: String::new(), + track: format!("Track {index}"), + variants: Vec::new(), + play_count: 1, + earliest: index as u64, + latest: index as u64, + }) + .collect::>(); + let batches = build_review_batches(&rows); + + assert_eq!(batches.len(), 3); + assert_eq!( + batches.iter().map(|batch| batch.page).collect::>(), + [1, 2, 3] + ); + assert_eq!(batches[0].source_ids.len(), LASTFM_REVIEW_BATCH_SIZE); + assert_eq!(batches[1].source_ids.len(), LASTFM_REVIEW_BATCH_SIZE); + assert_eq!(batches[2].source_ids.len(), 5); + assert_eq!(batches[0].source_ids[0], "source-0"); + assert_eq!(batches[1].source_ids[0], "source-100"); + assert_eq!(batches[2].source_ids[0], "source-200"); + } + + #[tokio::test] + async fn split_batch_default_options_are_local_and_each_batch_can_commit() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 1000); + aggregate_scrobbles( + &mut session.rows, + &(0..205) + .map(|index| scrobble("Artist", "Album", &format!("Track {index}"), index + 1)) + .collect::>(), + ); + session.phase = ImportPhase::Review; + service.save(session).await.unwrap(); + + for batch_id in 1..=3 { + let page = service.page(batch_id, "Artist", "Album").await.unwrap(); + let source_ids = page + .rows + .iter() + .map(|item| item.source.stable_id.clone()) + .collect::>(); + assert_eq!(page.options.selected_track_ids, source_ids); + let selected_ids = page + .options + .selected_track_ids + .iter() + .cloned() + .collect::>(); + service + .commit_rows( + "user", + "spotify", + batch_id, + &selected_ids, + "Artist", + "Album", + page.options, + ) + .await + .unwrap(); + } + + assert_eq!(service.snapshot().await.unwrap().phase, ImportPhase::Done); + } + + #[tokio::test] + async fn queue_pages_are_bounded_and_validate_cursor_and_limit() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 1000); + aggregate_scrobbles( + &mut session.rows, + &(0..205) + .map(|index| scrobble("Artist", "Album", &format!("Track {index}"), index + 1)) + .collect::>(), + ); + session.phase = ImportPhase::Review; + service.save(session).await.unwrap(); + + let first = service.queue_page(0, 2).await.unwrap(); + assert_eq!(first.total, 3); + assert_eq!(first.items.len(), 2); + assert_eq!( + first + .items + .iter() + .map(|item| item.source_count) + .collect::>(), + [100, 100] + ); + assert_eq!(first.next_cursor, Some(2)); + let second = service + .queue_page(first.next_cursor.unwrap(), 2) + .await + .unwrap(); + assert_eq!( + second + .items + .iter() + .map(|item| item.source_count) + .collect::>(), + [5] + ); + assert_eq!(second.next_cursor, None); + assert!(service.queue_page(0, 0).await.is_err()); + assert!(service + .queue_page(0, LASTFM_QUEUE_PAGE_LIMIT + 1) + .await + .is_err()); + assert!(service.queue_page(first.total + 1, 1).await.is_err()); + } + + #[test] + fn accept_all_entity_counts_are_unique_across_batches() { + let mut session = + LastFmImportSessionV2::new_with_defaults("user".into(), 100, ImportDefaults::default()); + aggregate_scrobbles( + &mut session.rows, + &[ + scrobble("Artist A", "Album A", "One", 1), + scrobble("Artist B", "Album B", "Two", 2), + ], + ); + for row in &session.rows { + session.page_options.insert( + format!("{}\u{1f}{}", row.artist, row.album), + PageOptions { + whole_album: true, + selected_track_ids: BTreeSet::from([row.stable_id.clone()]), + ..PageOptions::default() + }, + ); + session.matches.insert( + row.stable_id.clone(), + MatchResult { + source_id: row.stable_id.clone(), + search_term: "album".into(), + confidence: Some(Confidence::Exact), + selected_uri: Some("spotify:album:shared".into()), + candidates: Vec::new(), + track_matches: BTreeMap::new(), + }, + ); + } + let (albums, tracks) = accept_all_entity_uris(&session); + assert_eq!(albums, BTreeSet::from(["spotify:album:shared".into()])); + assert!(tracks.is_empty()); + + for options in session.page_options.values_mut() { + options.whole_album = false; + } + for row in &session.rows { + session + .matches + .get_mut(&row.stable_id) + .unwrap() + .track_matches = + BTreeMap::from([(row.stable_id.clone(), "spotify:track:shared".into())]); + } + let (albums, tracks) = accept_all_entity_uris(&session); + assert!(albums.is_empty()); + assert_eq!(tracks, BTreeSet::from(["spotify:track:shared".into()])); + } + + #[tokio::test] + async fn lazy_coordinator_shares_duplicate_opens_and_does_not_prefetch_adjacent_batches() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + let mut session = + LastFmImportSessionV2::new_with_defaults("user".into(), 100, ImportDefaults::default()); + aggregate_scrobbles( + &mut session.rows, + &[ + scrobble("Artist A", "Album A", "One", 1), + scrobble("Artist B", "Album B", "Two", 2), + ], + ); + session.phase = ImportPhase::Review; + service.save(session).await.unwrap(); + + let gate = Arc::new(tokio::sync::Mutex::new(())); + let searches = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let first_service = Arc::clone(&service); + let first_gate = Arc::clone(&gate); + let first_searches = Arc::clone(&searches); + let first = async move { + lazy_match_page_with_search( + first_service.as_ref(), + first_gate.as_ref(), + 1, + "Artist A", + "Album A", + || async { Ok(("user".into(), "spotify".into())) }, + move |rows| { + let results = rows + .into_iter() + .map(|row| MatchResult { + source_id: row.stable_id.clone(), + search_term: row.track.clone(), + confidence: Some(Confidence::Exact), + selected_uri: Some("spotify:track:shared".into()), + candidates: Vec::new(), + track_matches: BTreeMap::from([( + row.stable_id, + "spotify:track:shared".into(), + )]), + }) + .collect(); + let searches = Arc::clone(&first_searches); + async move { + searches.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + tokio::task::yield_now().await; + Ok(results) + } + }, + ) + .await + }; + let second_service = Arc::clone(&service); + let second_gate = Arc::clone(&gate); + let second_searches = Arc::clone(&searches); + let second = async move { + lazy_match_page_with_search( + second_service.as_ref(), + second_gate.as_ref(), + 1, + "Artist A", + "Album A", + || async { Ok(("user".into(), "spotify".into())) }, + move |rows| { + let results = rows + .into_iter() + .map(|row| MatchResult { + source_id: row.stable_id.clone(), + search_term: row.track.clone(), + confidence: Some(Confidence::Exact), + selected_uri: Some("spotify:track:shared".into()), + candidates: Vec::new(), + track_matches: BTreeMap::from([( + row.stable_id, + "spotify:track:shared".into(), + )]), + }) + .collect(); + let searches = Arc::clone(&second_searches); + async move { + searches.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + tokio::task::yield_now().await; + Ok(results) + } + }, + ) + .await + }; + let (first, second) = tokio::join!(first, second); + assert!(first.unwrap().is_some()); + assert!(second.unwrap().is_some()); + assert_eq!(searches.load(std::sync::atomic::Ordering::SeqCst), 1); + + let session = service.snapshot().await.unwrap(); + assert_eq!(session.spotify_account_id.as_deref(), Some("spotify")); + assert_eq!(session.matches.len(), 1); + assert!(!session.matches.contains_key(&session.rows[1].stable_id)); + + let cached_searches = Arc::clone(&searches); + let cached = lazy_match_page_with_search( + &service, + gate.as_ref(), + 1, + "Artist A", + "Album A", + || async { Ok(("user".into(), "spotify".into())) }, + move |_| async move { + cached_searches.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Ok(Vec::new()) + }, + ) + .await + .unwrap(); + assert!(cached.is_some()); + assert_eq!(searches.load(std::sync::atomic::Ordering::SeqCst), 1); + } + + #[tokio::test] + async fn lazy_coordinator_suspends_before_persisting_after_account_mismatch() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + let mut session = + LastFmImportSessionV2::new_with_defaults("user".into(), 100, ImportDefaults::default()); + aggregate_scrobbles( + &mut session.rows, + &[scrobble("Artist", "Album", "Track", 1)], + ); + session.phase = ImportPhase::Review; + service.save(session).await.unwrap(); + + let gate = tokio::sync::Mutex::new(()); + let account_checks = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let searches = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let error = lazy_match_page_with_search( + &service, + &gate, + 1, + "Artist", + "Album", + { + let account_checks = Arc::clone(&account_checks); + move || { + let account = account_checks.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + async move { + Ok(( + "user".into(), + if account == 0 { + "spotify-a" + } else { + "spotify-b" + } + .into(), + )) + } + } }, - ); - } -} + { + let searches = Arc::clone(&searches); + move |rows| { + searches.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + async move { + Ok(rows + .into_iter() + .map(|row| MatchResult { + source_id: row.stable_id, + search_term: row.track, + confidence: Some(Confidence::Exact), + selected_uri: Some("spotify:track:target".into()), + candidates: Vec::new(), + track_matches: BTreeMap::new(), + }) + .collect()) + } + } + }, + ) + .await + .unwrap_err(); -#[cfg(test)] -mod tests { - use super::*; - use crate::store::{SavedAlbumRecord, SpotifyLibraryState}; - use retune_core::model::SourceId; - #[cfg(unix)] - use std::os::unix::fs::PermissionsExt; - use std::{fs, time::Duration}; + assert!(error.contains("changed while matching")); + assert_eq!(account_checks.load(std::sync::atomic::Ordering::SeqCst), 2); + assert_eq!(searches.load(std::sync::atomic::Ordering::SeqCst), 1); + let session = service.snapshot().await.unwrap(); + assert_eq!(session.phase, ImportPhase::Suspended); + assert!(session.matches.is_empty()); + assert!(session.spotify_account_id.is_none()); + } - fn response(entries: Value) -> Value { - serde_json::json!({ - "recenttracks": { - "track": entries, - "@attr": {"page": "2", "totalPages": "4", "total": "601"} + #[tokio::test] + async fn accept_all_preparation_is_sequential_and_dedupes_entities() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + let mut session = + LastFmImportSessionV2::new_with_defaults("user".into(), 100, ImportDefaults::default()); + aggregate_scrobbles( + &mut session.rows, + &[ + scrobble("Artist A", "Album A", "One", 1), + scrobble("Artist B", "Album B", "Two", 2), + scrobble("Artist C", "Album C", "Three", 3), + ], + ); + session.phase = ImportPhase::Review; + service.save(session).await.unwrap(); + let order = Arc::new(tokio::sync::Mutex::new(Vec::new())); + let summary = prepare_accept_all_batches(&service, { + let service = Arc::clone(&service); + let order = Arc::clone(&order); + move |batch_id, artist, album| { + let service = Arc::clone(&service); + let order = Arc::clone(&order); + async move { + order.lock().await.push((artist.clone(), album.clone())); + let session = service.snapshot().await.unwrap(); + let results = session + .rows + .iter() + .filter(|row| row.artist == artist && row.album == album) + .map(|row| MatchResult { + source_id: row.stable_id.clone(), + search_term: row.track.clone(), + confidence: Some(Confidence::Exact), + selected_uri: Some("spotify:track:shared".into()), + candidates: Vec::new(), + track_matches: BTreeMap::from([( + row.stable_id.clone(), + "spotify:track:shared".into(), + )]), + }) + .collect(); + service + .set_matches("user", "spotify", batch_id, results) + .await + } } }) + .await + .unwrap(); + + assert_eq!(summary.album_entities, 0); + assert_eq!(summary.track_entities, 1); + assert_eq!( + *order.lock().await, + vec![ + ("Artist A".into(), "Album A".into()), + ("Artist B".into(), "Album B".into()), + ("Artist C".into(), "Album C".into()), + ] + ); } - fn scrobble(artist: &str, album: &str, track: &str, timestamp: u64) -> ParsedScrobble { - ParsedScrobble { - artist: artist.into(), - album: album.into(), - track: track.into(), - timestamp, - } + #[tokio::test] + async fn one_lazy_batch_uses_only_its_spotify_requests() { + let client = retune_spotify::client::fake_client( + [retune_spotify::client::Response::json( + 200, + serde_json::json!({ + "tracks": { + "items": [{ + "uri": "spotify:track:one", + "name": "One", + "artists": [{"id": "artist", "name": "Artist"}], + "album": { + "id": "album", + "uri": "spotify:album:album", + "name": "Album" + } + }], + "next": null, + "total": 1 + } + }), + )], + "", + ); + let rows = vec![SourceRow { + stable_id: "artist\u{1f}\u{1f}one".into(), + artist: "Artist".into(), + album: String::new(), + track: "One".into(), + variants: Vec::new(), + play_count: 1, + earliest: 1, + latest: 1, + }]; + let matches = match_batch(&client, "Artist", "", &rows).await.unwrap(); + assert_eq!(matches.len(), 1); + let requests = client.transport().requests(); + assert_eq!(requests.len(), 1); + assert!(requests[0].url.contains("/search?")); + assert!(requests[0].url.contains("type=track")); } #[test] @@ -2991,15 +5270,14 @@ mod tests { } #[test] - fn setup_state_view_reports_identity_and_review_only_remaining() { - let identity = ("rianjs".to_owned(), "spotify-user".to_owned()); - let setup = state_view_with_identity(None, Some(&identity)); + fn setup_state_view_reports_review_only_remaining() { + let setup = state_view(None); assert_eq!(setup.phase, None); - assert_eq!(setup.username.as_deref(), Some("rianjs")); - assert_eq!(setup.spotify_account_id.as_deref(), Some("spotify-user")); + assert_eq!(setup.username, None); + assert_eq!(setup.spotify_account_id, None); assert_eq!(setup.remaining, 0); - let mut session = LastFmImportSessionV1::new("rianjs".into(), "spotify-user".into(), 10); + let mut session = LastFmImportSessionV2::new("rianjs".into(), "spotify-user".into(), 10); aggregate_scrobbles( &mut session.rows, &[scrobble("Artist", "Album", "Track", 10)], @@ -3028,10 +5306,10 @@ mod tests { } #[tokio::test] - async fn page_fuzzy_groups_only_include_rows_selected_for_import() { + async fn page_fuzzy_groups_stay_inside_the_requested_batch() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - let mut session = LastFmImportSessionV1::new("user".into(), "spotify".into(), 10); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 10); aggregate_scrobbles( &mut session.rows, &[ @@ -3111,30 +5389,133 @@ mod tests { } service.save(session).await.unwrap(); - let page = service.page("Artist", "Selected").await.unwrap(); - let included = page - .fuzzy_groups - .get(&target) + let session = service.snapshot().await.unwrap(); + let selected_page = review_batches(&session) + .into_iter() + .find(|batch| { + batch_rows(batch, &source_row_map(&session)) + .iter() + .any(|row| row.album == "Selected") + }) .unwrap() + .page; + let page = service + .page(selected_page, "Artist", "Selected") + .await + .unwrap(); + assert_eq!( + page.rows + .iter() + .map(|item| item.source.album.as_str()) + .collect::>(), + BTreeSet::from(["Selected"]) + ); + assert!(!page.fuzzy_groups.contains_key(&target)); + assert!(!page.locked_count_modes.contains(&target)); + } + + #[tokio::test] + async fn page_projects_count_modes_to_visible_fuzzy_targets() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 10); + aggregate_scrobbles( + &mut session.rows, + &[ + scrobble("Artist", "Hidden", "Track", 1), + scrobble("Artist", "Visible", "Track", 2), + scrobble("Artist", "Visible", "Track (Live)", 3), + ], + ); + let visible_ids = session + .rows .iter() - .map(|row| row.album.clone()) - .collect::>(); + .filter(|row| row.album == "Visible") + .map(|row| row.stable_id.clone()) + .collect::>(); + let hidden_id = session + .rows + .iter() + .find(|row| row.album == "Hidden") + .unwrap() + .stable_id + .clone(); + let visible_target = "spotify:track:visible".to_owned(); + let hidden_target = "spotify:track:hidden".to_owned(); + for (source_id, target) in visible_ids + .iter() + .map(|id| (id, &visible_target)) + .chain(std::iter::once((&hidden_id, &hidden_target))) + { + session.matches.insert( + source_id.clone(), + MatchResult { + source_id: source_id.clone(), + search_term: String::new(), + confidence: Some(Confidence::Exact), + selected_uri: Some(target.clone()), + candidates: Vec::new(), + track_matches: BTreeMap::from([(source_id.clone(), target.clone())]), + }, + ); + } + session.decisions.insert( + visible_ids[0].clone(), + RowDecision { + status: RowStatus::Done, + excluded: false, + }, + ); + session.decisions.insert( + hidden_id, + RowDecision { + status: RowStatus::Done, + excluded: false, + }, + ); + session + .count_modes + .insert(visible_target.clone(), CountMode::Overwrite); + session.count_modes.insert(hidden_target, CountMode::Zero); + session.page_options.insert( + "Artist\u{1f}Visible".into(), + PageOptions { + selected_track_ids: visible_ids.iter().cloned().collect(), + ..PageOptions::default() + }, + ); + session.phase = ImportPhase::Review; + service.save(session).await.unwrap(); + + let session = service.snapshot().await.unwrap(); + let visible_page = review_batches(&session) + .into_iter() + .find(|batch| { + batch_rows(batch, &source_row_map(&session)) + .iter() + .any(|row| row.album == "Visible") + }) + .unwrap(); + let page = service + .page(visible_page.page, "Artist", "Visible") + .await + .unwrap(); + assert_eq!( + page.fuzzy_groups.keys().cloned().collect::>(), + BTreeSet::from([visible_target.clone()]) + ); assert_eq!( - included, - BTreeSet::from([ - "Done".to_owned(), - "Selected".to_owned(), - "Skipped".to_owned(), - ]) + page.count_modes, + BTreeMap::from([(visible_target.clone(), CountMode::Overwrite)]) ); - assert!(page.locked_count_modes.contains(&target)); + assert_eq!(page.locked_count_modes, BTreeSet::from([visible_target])); } #[tokio::test] async fn count_mode_change_is_rejected_after_target_is_done() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - let mut session = LastFmImportSessionV1::new("user".into(), "spotify".into(), 10); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 10); aggregate_scrobbles( &mut session.rows, &[scrobble("Artist", "Album", "Track", 1)], @@ -3181,7 +5562,7 @@ mod tests { #[test] fn target_count_mode_is_session_scoped_across_pages_and_persisted() { - let mut session = LastFmImportSessionV1::new("user".into(), "spotify".into(), 10); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 10); aggregate_scrobbles( &mut session.rows, &[ @@ -3285,7 +5666,7 @@ mod tests { #[test] fn fuzzy_strategy_remains_independent_per_target() { - let mut session = LastFmImportSessionV1::new("user".into(), "spotify".into(), 10); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 10); aggregate_scrobbles( &mut session.rows, &[ @@ -3635,7 +6016,7 @@ mod tests { #[test] fn review_actions_cascade_and_remaining_count_is_durable() { - let mut session = LastFmImportSessionV1::new("user".into(), "spotify".into(), 10); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 10); aggregate_scrobbles( &mut session.rows, &[ @@ -3666,10 +6047,7 @@ mod tests { async fn fully_excluded_review_action_reaches_done_and_has_view_only_queue_status() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - service - .start_or_resume("lastfm-user", "spotify-user", 500, None) - .await - .unwrap(); + start_bound(&service, "lastfm-user", "spotify-user", 500).await; service .checkpoint_page( 1, @@ -3685,6 +6063,7 @@ mod tests { ) .await .unwrap(); + service.aggregate_cached(None).await.unwrap(); let mut session = service.snapshot().await.unwrap(); session.phase = ImportPhase::Review; service.save(session.clone()).await.unwrap(); @@ -3693,6 +6072,7 @@ mod tests { .review_action( "lastfm-user", "spotify-user", + 1, &row.stable_id, "exclude", "A", @@ -3708,11 +6088,90 @@ mod tests { assert_eq!(session.remaining(), 0); } + #[tokio::test] + async fn album_review_actions_cascade_across_split_batches_and_restore_from_any_batch() { + let dir = tempfile::tempdir().unwrap(); + let service = Service::new(dir.path()); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 1000); + aggregate_scrobbles( + &mut session.rows, + &(0..205) + .map(|index| scrobble("Artist", "Album", &format!("Track {index}"), index + 1)) + .collect::>(), + ); + session.phase = ImportPhase::Review; + service.save(session.clone()).await.unwrap(); + + service + .review_action( + "user", + "spotify", + 1, + &session.rows[0].stable_id, + "ignore-album", + "Artist", + "Album", + ) + .await + .unwrap(); + let queue = service + .queue_page(0, LASTFM_QUEUE_PAGE_LIMIT) + .await + .unwrap() + .items; + assert_eq!(queue.len(), 3); + assert!(queue + .iter() + .all(|item| item.status == Some(QueueStatus::IgnoredAlbum) && !item.remaining)); + + service + .review_action( + "user", + "spotify", + 2, + &session.rows[100].stable_id, + "restore", + "Artist", + "Album", + ) + .await + .unwrap(); + let queue = service + .queue_page(0, LASTFM_QUEUE_PAGE_LIMIT) + .await + .unwrap() + .items; + assert!(queue + .iter() + .all(|item| item.status.is_none() && item.remaining)); + + service + .review_action( + "user", + "spotify", + 3, + &session.rows[200].stable_id, + "skip-album", + "Artist", + "Album", + ) + .await + .unwrap(); + let queue = service + .queue_page(0, LASTFM_QUEUE_PAGE_LIMIT) + .await + .unwrap() + .items; + assert!(queue + .iter() + .all(|item| item.status == Some(QueueStatus::Skipped) && item.remaining)); + } + #[tokio::test] async fn owned_review_mutations_reject_mismatch_and_suspension() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - let mut session = LastFmImportSessionV1::new("user".into(), "spotify".into(), 10); + let mut session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 10); session.phase = ImportPhase::Review; service.save(session).await.unwrap(); @@ -3725,11 +6184,18 @@ mod tests { service.save(session).await.unwrap(); assert!(service - .review_action("user", "spotify", "id", "exclude", "Artist", "Album") + .review_action("user", "spotify", 1, "id", "exclude", "Artist", "Album") .await .is_err()); assert!(service - .update_options("user", "spotify", "Artist", "Album", PageOptions::default(),) + .update_options( + "user", + "spotify", + 1, + "Artist", + "Album", + PageOptions::default(), + ) .await .is_err()); assert!(service @@ -3744,6 +6210,7 @@ mod tests { .set_match( "user", "spotify", + 1, MatchResult { source_id: "id".into(), search_term: "track".into(), @@ -3756,7 +6223,7 @@ mod tests { .await .is_err()); assert!(service - .select_match("user", "spotify", "id", "spotify:track:target") + .select_match("user", "spotify", 1, "id", "spotify:track:target") .await .is_err()); @@ -3771,7 +6238,7 @@ mod tests { fn persistence_round_trip_quarantines_corrupt_unknown_and_rejects_oversize() { let dir = tempfile::tempdir().unwrap(); let store = ImportSessionStore::new(dir.path()); - let session = LastFmImportSessionV1::new("user".into(), "spotify".into(), 42); + let session = LastFmImportSessionV2::new("user".into(), "spotify".into(), 42); store.save(&session).unwrap(); assert_eq!(store.load().unwrap(), Some(session.clone())); #[cfg(unix)] @@ -3812,13 +6279,13 @@ mod tests { .to_string_lossy() .contains("quarantine"))); - let mut unknown = LastFmImportSessionV1::new("user".into(), "spotify".into(), 42); + let mut unknown = LastFmImportSessionV2::new("user".into(), "spotify".into(), 42); unknown.version = 99; store.save(&unknown).unwrap(); assert_eq!(store.load().unwrap(), None); assert!(fs::read_dir(dir.path()).unwrap().count() >= 2); - let mut too_large = LastFmImportSessionV1::new("user".into(), "spotify".into(), 42); + let mut too_large = LastFmImportSessionV2::new("user".into(), "spotify".into(), 42); too_large.rows.push(SourceRow { stable_id: "x".into(), artist: "a".into(), @@ -3844,48 +6311,40 @@ mod tests { async fn page_checkpoint_resume_is_idempotent_and_account_mismatch_suspends() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); + start_bound(&service, "lastfm-user", "spotify-user", 500).await; + service.set_metadata(2, 2).await.unwrap(); + let parsed = parsed_page(2, 2, vec![scrobble("Artist", "Album", "Track", 10)]); + service.checkpoint_page(2, &parsed).await.unwrap(); + service.checkpoint_page(2, &parsed).await.unwrap(); service - .start_or_resume("lastfm-user", "spotify-user", 500, None) + .checkpoint_page(1, &parsed_page(1, 2, Vec::new())) .await .unwrap(); - let parsed = ParsedRecentTracksPage { - page: 1, - total_pages: Some(2), - total: Some(2), - tracks: vec![scrobble("Artist", "Album", "Track", 10)], - ..ParsedRecentTracksPage::default() - }; - service.checkpoint_page(1, &parsed).await.unwrap(); - service.checkpoint_page(1, &parsed).await.unwrap(); + service.aggregate_cached(None).await.unwrap(); let resumed = Service::new(dir.path()); let session = resumed.snapshot().await.unwrap(); - assert_eq!(session.next_page, 2); + assert_eq!(session.next_page, 0); assert_eq!(session.rows.len(), 1); assert_eq!(session.included_scrobbles, 1); - let mismatch = resumed - .start_or_resume("other-user", "spotify-user", 600, None) - .await; + let mismatch = resumed.start_or_resume("other-user", 600, None).await; assert!(mismatch.is_err()); assert_eq!( resumed.snapshot().await.unwrap().phase, ImportPhase::Suspended ); let resumed_for_owner = resumed - .start_or_resume("lastfm-user", "spotify-user", 600, None) + .start_or_resume("lastfm-user", 600, None) .await .unwrap(); - assert_eq!(resumed_for_owner.phase, Some(ImportPhase::Downloading)); + assert_eq!(resumed_for_owner.phase, Some(ImportPhase::Review)); } #[tokio::test] async fn search_terms_preference_round_trips_on_resume() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - service - .start_or_resume("lastfm-user", "spotify-user", 500, None) - .await - .unwrap(); + start_bound(&service, "lastfm-user", "spotify-user", 500).await; let mut review_session = service.snapshot().await.unwrap(); review_session.phase = ImportPhase::Review; service.save(review_session).await.unwrap(); @@ -3902,10 +6361,7 @@ mod tests { async fn overlapping_mutations_preserve_memory_and_disk_changes() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - service - .start_or_resume("lastfm-user", "spotify-user", 500, None) - .await - .unwrap(); + start_bound(&service, "lastfm-user", "spotify-user", 500).await; let mut review_session = service.snapshot().await.unwrap(); review_session.phase = ImportPhase::Review; service.save(review_session).await.unwrap(); @@ -3939,10 +6395,7 @@ mod tests { async fn failed_blocking_persistence_does_not_commit_live_mutation() { let dir = tempfile::tempdir().unwrap(); let mut service = Service::new(dir.path()); - service - .start_or_resume("lastfm-user", "spotify-user", 500, None) - .await - .unwrap(); + start_bound(&service, "lastfm-user", "spotify-user", 500).await; let mut review_session = service.snapshot().await.unwrap(); review_session.phase = ImportPhase::Review; service.save(review_session).await.unwrap(); @@ -3964,13 +6417,10 @@ mod tests { } #[tokio::test] - async fn matching_checkpoint_and_finalization_cannot_write_through_suspension() { + async fn matching_cannot_write_through_suspension() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - service - .start_or_resume("lastfm-user", "spotify-user", 500, None) - .await - .unwrap(); + start_bound(&service, "lastfm-user", "spotify-user", 500).await; service .checkpoint_page( 1, @@ -3985,9 +6435,10 @@ mod tests { .unwrap(); service.suspend_for_account_mismatch().await.unwrap(); let result = service - .set_matches_during_matching( + .set_matches( "lastfm-user", "spotify-user", + 1, vec![MatchResult { source_id: "artist\u{1f}album\u{1f}track".into(), search_term: "track search".into(), @@ -3999,10 +6450,6 @@ mod tests { ) .await; assert!(result.is_err()); - assert!(service - .finish_matching_if_current("lastfm-user", "spotify-user") - .await - .is_err()); let session = service.snapshot().await.unwrap(); assert_eq!(session.phase, ImportPhase::Suspended); assert!(session.matches.is_empty()); @@ -4012,10 +6459,7 @@ mod tests { async fn suspended_reads_are_redacted_and_empty() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - service - .start_or_resume("prior-user", "prior-spotify", 500, None) - .await - .unwrap(); + start_bound(&service, "prior-user", "prior-spotify", 500).await; service .checkpoint_page( 1, @@ -4034,47 +6478,48 @@ mod tests { assert_eq!(state.username, None); assert_eq!(state.spotify_account_id, None); assert_eq!(state.remaining, 0); - assert!(service.queue().await.is_empty()); - assert!(service.page("Prior Artist", "Prior Album").await.is_none()); + assert!(service + .queue_page(0, LASTFM_QUEUE_PAGE_LIMIT) + .await + .unwrap() + .items + .is_empty()); + assert!(service + .page(1, "Prior Artist", "Prior Album") + .await + .is_none()); } #[tokio::test] async fn mismatched_pages_do_not_advance_and_page_batches_are_compact() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - service - .start_or_resume("lastfm-user", "spotify-user", 500, None) - .await - .unwrap(); - let mismatched = ParsedRecentTracksPage { - page: 2, - tracks: vec![scrobble("Artist", "Album", "Track", 10)], - ..ParsedRecentTracksPage::default() - }; + start_bound(&service, "lastfm-user", "spotify-user", 500).await; + service.set_metadata(2, 1).await.unwrap(); + let mismatched = parsed_page(2, 2, vec![scrobble("Artist", "Album", "Track", 10)]); assert!(service.checkpoint_page(1, &mismatched).await.is_err()); - assert_eq!(service.snapshot().await.unwrap().next_page, 1); - let duplicate_page = ParsedRecentTracksPage { - page: 1, - tracks: vec![ + assert_eq!(service.snapshot().await.unwrap().next_page, 2); + service.checkpoint_page(2, &mismatched).await.unwrap(); + let duplicate_page = parsed_page( + 1, + 2, + vec![ scrobble("Artist", "Album", "Track", 10), scrobble("artist", "album", "track", 20), ], - ..ParsedRecentTracksPage::default() - }; + ); service.checkpoint_page(1, &duplicate_page).await.unwrap(); + service.aggregate_cached(None).await.unwrap(); let session = service.snapshot().await.unwrap(); assert_eq!(session.batches[0].source_ids.len(), 1); - assert_eq!(session.next_page, 2); + assert_eq!(session.next_page, 0); } #[tokio::test] async fn queue_reports_exact_entity_counts_for_current_page_choices() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - service - .start_or_resume("lastfm-user", "spotify-user", 500, None) - .await - .unwrap(); + start_bound(&service, "lastfm-user", "spotify-user", 500).await; let parsed = ParsedRecentTracksPage { page: 1, total_pages: Some(1), @@ -4086,6 +6531,7 @@ mod tests { ..ParsedRecentTracksPage::default() }; service.checkpoint_page(1, &parsed).await.unwrap(); + service.aggregate_cached(None).await.unwrap(); let mut review_session = service.snapshot().await.unwrap(); review_session.phase = ImportPhase::Review; service.save(review_session).await.unwrap(); @@ -4098,6 +6544,7 @@ mod tests { .set_match( "lastfm-user", "spotify-user", + 1, MatchResult { source_id: row.stable_id.clone(), search_term: "album search".into(), @@ -4119,13 +6566,18 @@ mod tests { .await .unwrap(); } - let queue = service.queue().await; + let queue = service + .queue_page(0, LASTFM_QUEUE_PAGE_LIMIT) + .await + .unwrap() + .items; assert_eq!((queue[0].album_entities, queue[0].track_entities), (0, 2)); service .update_options( "lastfm-user", "spotify-user", + 1, "Artist", "Album", PageOptions { @@ -4136,7 +6588,11 @@ mod tests { ) .await .unwrap(); - let queue = service.queue().await; + let queue = service + .queue_page(0, LASTFM_QUEUE_PAGE_LIMIT) + .await + .unwrap() + .items; assert_eq!((queue[0].album_entities, queue[0].track_entities), (1, 0)); let selected_track_ids = rows @@ -4147,6 +6603,7 @@ mod tests { .update_options( "lastfm-user", "spotify-user", + 1, "Artist", "Album", PageOptions { @@ -4157,13 +6614,18 @@ mod tests { ) .await .unwrap(); - let queue = service.queue().await; + let queue = service + .queue_page(0, LASTFM_QUEUE_PAGE_LIMIT) + .await + .unwrap() + .items; assert_eq!((queue[0].album_entities, queue[0].track_entities), (0, 2)); service .update_options( "lastfm-user", "spotify-user", + 1, "Artist", "Album", PageOptions { @@ -4174,7 +6636,11 @@ mod tests { ) .await .unwrap(); - let queue = service.queue().await; + let queue = service + .queue_page(0, LASTFM_QUEUE_PAGE_LIMIT) + .await + .unwrap() + .items; assert_eq!((queue[0].album_entities, queue[0].track_entities), (0, 0)); } @@ -4182,10 +6648,7 @@ mod tests { async fn selecting_an_album_candidate_remaps_every_related_source_track() { let dir = tempfile::tempdir().unwrap(); let service = Service::new(dir.path()); - service - .start_or_resume("lastfm-user", "spotify-user", 500, None) - .await - .unwrap(); + start_bound(&service, "lastfm-user", "spotify-user", 500).await; let parsed = ParsedRecentTracksPage { page: 1, total_pages: Some(1), @@ -4196,6 +6659,7 @@ mod tests { ..ParsedRecentTracksPage::default() }; service.checkpoint_page(1, &parsed).await.unwrap(); + service.aggregate_cached(None).await.unwrap(); let mut review_session = service.snapshot().await.unwrap(); review_session.phase = ImportPhase::Review; service.save(review_session).await.unwrap(); @@ -4243,6 +6707,7 @@ mod tests { .set_match( "lastfm-user", "spotify-user", + 1, MatchResult { source_id: row.stable_id.clone(), search_term: "album search".into(), @@ -4260,6 +6725,7 @@ mod tests { .select_match( "lastfm-user", "spotify-user", + 1, &rows[0].stable_id, "spotify:album:new", ) @@ -4282,6 +6748,7 @@ mod tests { .select_match( "lastfm-user", "spotify-user", + 1, &first_id, "spotify:track:rematched", ) diff --git a/apps/desktop/src-tauri/src/lib.rs b/apps/desktop/src-tauri/src/lib.rs index ecf5d8b..e8aeacd 100644 --- a/apps/desktop/src-tauri/src/lib.rs +++ b/apps/desktop/src-tauri/src/lib.rs @@ -53,8 +53,8 @@ use retune_spotify::{ }; use serde::{Deserialize, Serialize}; use store::{ - BrowserPanes, FsOverlayStore, FsPlaylistStore, FsSettingsStore, FsSyncStore, OverlayStore, - Settings, SpotifyLibraryState, StoreError, Theme, + BrowserPanes, FsOverlayStore, FsPlaylistStore, FsSettingsStore, FsSyncStore, + LastFmScrobblingProfile, OverlayStore, Settings, SpotifyLibraryState, StoreError, Theme, }; use sync_orchestrator::SyncOrchestrator; use tauri::{ @@ -183,6 +183,8 @@ struct ExportSettings { sort_desc: bool, #[serde(default)] shuffle: bool, + #[serde(default)] + lastfm_scrobbling_profile: Option, } impl ExportSettings { @@ -203,10 +205,11 @@ impl ExportSettings { sort_column: settings.sort_column.clone(), sort_desc: settings.sort_desc, shuffle: settings.shuffle, + lastfm_scrobbling_profile: settings.lastfm_scrobbling_profile.clone(), } } - fn apply_to(self, settings: &mut Settings) { + fn apply_to(self, settings: &mut Settings) -> Result<(), String> { settings.theme = self.theme; settings.zoom = self.zoom; settings.zebra = self.zebra; @@ -222,7 +225,11 @@ impl ExportSettings { settings.sort_column = self.sort_column; settings.sort_desc = self.sort_desc; settings.shuffle = self.shuffle; + if self.lastfm_scrobbling_profile.is_some() { + settings.lastfm_scrobbling_profile = self.lastfm_scrobbling_profile; + } settings.normalize(); + settings.validate().map_err(|error| error.to_string()) } } @@ -693,6 +700,11 @@ async fn set_settings(app: tauri::AppHandle, mut settings: Settings) -> Result<( let client_id_changed = current.spotify_client_id != settings.spotify_client_id; settings.spotify_sync_completed = current.spotify_sync_completed; settings.last_full_sync = current.last_full_sync; + if settings.lastfm_scrobbling { + if let Some(username) = state.lastfm.state().await.username { + reconcile_lastfm_scrobbling_profile(&mut settings, &username, unix_now()); + } + } settings.validate().map_err(|error| error.to_string())?; let wants_local = settings.playback_backend == "local"; state.playback.set_local_requested(wants_local); @@ -722,14 +734,23 @@ async fn set_settings(app: tauri::AppHandle, mut settings: Settings) -> Result<( Ok(()) } -pub(crate) fn set_lastfm_scrobbling(app: &tauri::AppHandle, enabled: bool) -> Result<(), String> { +pub(crate) async fn set_lastfm_scrobbling( + app: &tauri::AppHandle, + enabled: bool, +) -> Result<(), String> { let state = app.state::(); - let mut settings = state + let current = state .settings .lock() .expect("settings mutex poisoned") .clone(); + let mut settings = current.clone(); settings.lastfm_scrobbling = enabled; + if enabled { + if let Some(username) = state.lastfm.state().await.username { + reconcile_lastfm_scrobbling_profile(&mut settings, &username, unix_now()); + } + } state .settings_store .save(&settings) @@ -739,6 +760,45 @@ pub(crate) fn set_lastfm_scrobbling(app: &tauri::AppHandle, enabled: bool) -> Re .map_err(|error| error.to_string()) } +fn reconcile_lastfm_scrobbling_profile(settings: &mut Settings, username: &str, now: u64) { + if username.trim().is_empty() + || settings + .lastfm_scrobbling_profile + .as_ref() + .is_some_and(|profile| profile.username == username) + { + return; + } + settings.lastfm_scrobbling_profile = Some(LastFmScrobblingProfile { + username: username.to_owned(), + started_at: now, + }); +} + +async fn history_cutoff_for_import(app: &tauri::AppHandle, username: &str) -> Result { + let state = app.state::(); + let current = state + .settings + .lock() + .expect("settings mutex poisoned") + .clone(); + let mut settings = current.clone(); + reconcile_lastfm_scrobbling_profile(&mut settings, username, unix_now()); + let cutoff = settings + .lastfm_scrobbling_profile + .as_ref() + .map(|profile| profile.started_at) + .ok_or_else(|| "Could not establish the Last.fm history cutoff.".to_string())?; + if settings != current { + state + .settings_store + .save(&settings) + .map_err(|error| error.to_string())?; + *state.settings.lock().expect("settings mutex poisoned") = settings; + } + Ok(cutoff) +} + async fn switch_to_local(state: &AppState, volume: u8) -> Result<(), String> { state .token_store @@ -2081,13 +2141,16 @@ fn import_with_settings( } let mut envelope: serde_json::Value = serde_json::from_slice(&json).map_err(|error| error.to_string())?; - let settings = envelope + let settings: Option = envelope .as_object_mut() .and_then(|object| object.remove("settings")) .filter(|_| restore) .map(serde_json::from_value) .transpose() .map_err(|error| error.to_string())?; + if let Some(settings) = &settings { + settings.clone().apply_to(&mut Settings::default())?; + } let playlists = envelope .as_object_mut() .and_then(|object| object.remove("playlists")) @@ -2180,7 +2243,7 @@ fn apply_export_settings( .lock() .expect("settings mutex poisoned") .clone(); - export_settings.apply_to(&mut settings); + export_settings.apply_to(&mut settings)?; state .settings_store .save(&settings) @@ -2279,6 +2342,7 @@ pub fn run() { lastfm_import::lastfm_import_change_track, lastfm_import::lastfm_import_change_album, lastfm_import::lastfm_import_apply, + lastfm_import::lastfm_import_prepare_accept_all, lastfm_import::lastfm_import_accept_all_page, diagnostics::load_diagnostics, diagnostics::email_diagnostics @@ -2416,8 +2480,11 @@ pub fn run() { }); lastfm.attach_app(app.handle().clone()); let lastfm_startup = Arc::clone(&lastfm); + let profile_app = app.handle().clone(); tauri::async_runtime::spawn(async move { lastfm_startup.set_enabled(lastfm_enabled).await; + let _ = set_lastfm_scrobbling(&profile_app, lastfm_enabled).await; + lastfm_import::resume_persisted_import(profile_app.clone()).await; }); let completion_app = app.handle().clone(); let lastfm = Arc::clone(&lastfm); @@ -3578,6 +3645,37 @@ mod tests { assert!(settings.spotify_sync_completed); } + #[test] + fn lastfm_profile_is_account_bound_and_survives_toggles() { + let mut settings = Settings::default(); + reconcile_lastfm_scrobbling_profile(&mut settings, "first-user", 10); + assert_eq!( + settings.lastfm_scrobbling_profile, + Some(LastFmScrobblingProfile { + username: "first-user".into(), + started_at: 10, + }) + ); + settings.lastfm_scrobbling = false; + reconcile_lastfm_scrobbling_profile(&mut settings, "first-user", 20); + assert_eq!( + settings + .lastfm_scrobbling_profile + .as_ref() + .unwrap() + .started_at, + 10 + ); + reconcile_lastfm_scrobbling_profile(&mut settings, "second-user", 30); + assert_eq!( + settings.lastfm_scrobbling_profile, + Some(LastFmScrobblingProfile { + username: "second-user".into(), + started_at: 30, + }) + ); + } + #[test] fn export_restore_round_trips_visual_settings_and_playlist_order() { let library = fixture::library(); @@ -3679,6 +3777,10 @@ mod tests { gapless: false, play_threshold_percent: 100, lastfm_scrobbling: true, + lastfm_scrobbling_profile: Some(LastFmScrobblingProfile { + username: "exported-user".into(), + started_at: 1786804381, + }), }; let plain = export_with_settings(&library, &exported, &playlists).unwrap(); // Gzip the export ourselves so import's GzDecoder path stays covered @@ -3695,7 +3797,7 @@ mod tests { spotify_sync_completed: false, ..Settings::default() }; - visual.unwrap().apply_to(&mut restored); + visual.unwrap().apply_to(&mut restored).unwrap(); assert_eq!(restored_library, library); assert_eq!(restored_playlists, Some(playlists)); @@ -3725,6 +3827,10 @@ mod tests { assert!(restored.auto_add_spotify_library); assert!(restored.auto_connect); assert!(!restored.spotify_sync_completed); + assert_eq!( + restored.lastfm_scrobbling_profile, + exported.lastfm_scrobbling_profile + ); } #[test] @@ -3737,7 +3843,7 @@ mod tests { alb: true, }; - visual.apply_to(&mut settings); + visual.apply_to(&mut settings).unwrap(); assert_eq!( settings.browser_panes, @@ -3749,6 +3855,40 @@ mod tests { ); } + #[test] + fn export_restore_rejects_invalid_lastfm_profile() { + for profile in [ + LastFmScrobblingProfile { + username: " ".into(), + started_at: 1, + }, + LastFmScrobblingProfile { + username: "user".into(), + started_at: 0, + }, + ] { + let mut settings = Settings::default(); + let mut export = ExportSettings::from_settings(&settings); + export.lastfm_scrobbling_profile = Some(profile); + assert!(export.apply_to(&mut settings).is_err()); + } + + let invalid = Settings { + lastfm_scrobbling_profile: Some(LastFmScrobblingProfile { + username: " ".into(), + started_at: 1, + }), + ..Settings::default() + }; + let bytes = export_with_settings( + &fixture::library(), + &invalid, + &playlists::PlaylistCache { playlists: vec![] }, + ) + .unwrap(); + assert!(import_with_settings(&bytes, true).is_err()); + } + #[test] fn visual_settings_apply_normalizes_legacy_columns() { let mut json = @@ -3766,7 +3906,7 @@ mod tests { let visual: ExportSettings = serde_json::from_value(json).unwrap(); let mut settings = Settings::default(); - visual.apply_to(&mut settings); + visual.apply_to(&mut settings).unwrap(); assert_eq!( settings.column_order, diff --git a/apps/desktop/src-tauri/src/store.rs b/apps/desktop/src-tauri/src/store.rs index 2bae82f..f4e3d79 100644 --- a/apps/desktop/src-tauri/src/store.rs +++ b/apps/desktop/src-tauri/src/store.rs @@ -74,6 +74,13 @@ pub struct FsOverlayStore { path: PathBuf, } +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct LastFmScrobblingProfile { + pub username: String, + pub started_at: u64, +} + #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct Settings { @@ -128,6 +135,8 @@ pub struct Settings { pub play_threshold_percent: u8, #[serde(default = "default_true")] pub lastfm_scrobbling: bool, + #[serde(default)] + pub lastfm_scrobbling_profile: Option, } #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] @@ -210,6 +219,7 @@ impl Default for Settings { gapless: true, play_threshold_percent: default_play_threshold_percent(), lastfm_scrobbling: true, + lastfm_scrobbling_profile: None, } } } @@ -418,6 +428,15 @@ impl Settings { "settings streamingBitrate must be 96, 160, or 320", )); } + if self + .lastfm_scrobbling_profile + .as_ref() + .is_some_and(|profile| profile.username.trim().is_empty() || profile.started_at == 0) + { + return Err(StoreError::InvalidSettings( + "settings lastfmScrobblingProfile must have a username and positive startedAt", + )); + } Ok(()) } @@ -939,6 +958,10 @@ mod tests { gapless: false, play_threshold_percent: 75, lastfm_scrobbling: false, + lastfm_scrobbling_profile: Some(LastFmScrobblingProfile { + username: "rianjs".into(), + started_at: 42, + }), }; assert!(store.load().unwrap().is_none()); @@ -1511,6 +1534,38 @@ mod tests { assert!(settings.validate().is_err()); } + #[test] + fn lastfm_scrobbling_profile_is_validated_on_load_and_save() { + for profile in [ + LastFmScrobblingProfile { + username: " ".into(), + started_at: 1, + }, + LastFmScrobblingProfile { + username: "user".into(), + started_at: 0, + }, + ] { + let settings = Settings { + lastfm_scrobbling_profile: Some(profile.clone()), + ..Settings::default() + }; + assert!(settings.validate().is_err()); + + let dir = tempfile::tempdir().unwrap(); + let store = FsSettingsStore::new(dir.path()); + let mut json = serde_json::to_value(Settings::default()).unwrap(); + json["lastfmScrobblingProfile"] = serde_json::to_value(profile).unwrap(); + fs::write( + dir.path().join("settings.json"), + serde_json::to_vec(&json).unwrap(), + ) + .unwrap(); + assert!(store.load().is_err()); + assert!(store.save(&settings).is_err()); + } + } + #[test] fn streaming_bitrate_accepts_supported_qualities_only() { for streaming_bitrate in [96, 160, 320] { diff --git a/apps/desktop/src/LastFmImporter.tsx b/apps/desktop/src/LastFmImporter.tsx index 85674d2..0de661b 100644 --- a/apps/desktop/src/LastFmImporter.tsx +++ b/apps/desktop/src/LastFmImporter.tsx @@ -4,7 +4,7 @@ import { getCurrentWindow } from '@tauri-apps/api/window' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { ModalDialog } from './viewShared.tsx' import type { LastFmImportDefaults, Settings } from './types.ts' -import { applyCurrentImportPageResponse, downloadAction, excludedImportCount, importStatusText, isCurrentImportPageResponse, loadSelectedImportPage, nextRemainingImportQueue, pickerCandidates, pickerSelectedUri, resolveImportCount, restPendingImportCount, selectedImportCount, selectedImportTrackConfidence, showsImportRemaining, sortImportQueue, toggleImportRow, trackPickerQuery, validImportIntent, type CountMode, type ImportConfidence, type ImportPhase, type ImportPickerKind, type ImportQueueItem, type ImportSourceRow, type ReviewState } from './lastfmImportState.ts' +import { applyCurrentImportPageResponse, downloadAction, excludedImportCount, importEmptyPageMessage, importStatusText, isCurrentImportPageResponse, loadSelectedImportPage, nextRemainingImportQueue, pickerCandidates, pickerSelectedUri, resolveImportCount, restPendingImportCount, selectedImportCount, selectedImportTrackConfidence, showsImportRemaining, sortImportQueue, toggleImportRow, trackPickerQuery, validImportIntent, type CountMode, type ImportConfidence, type ImportPhase, type ImportPickerKind, type ImportQueueItem, type ImportQueuePage, type ImportSourceRow, type ReviewState } from './lastfmImportState.ts' import './lastfmImporter.css' type ImportStateView = { @@ -13,10 +13,9 @@ type ImportStateView = { spotifyAccountId: string | null nextPage: number totalPages: number | null + downloadedPages: number totalScrobbles: number includedScrobbles: number - matchedRows: number - matchTotal: number defaults: LastFmImportDefaults remaining: number retryableError: { message: string; attempt: number; retryable: boolean } | null @@ -25,13 +24,32 @@ type ImportStateView = { type AlbumCandidate = { uri: string; name: string; artist: string; relation: 'best-match' | 'same-songs' | 'superset' | null; trackUris: string[]; trackNames: string[]; trackArtists: string[]; trackAlbums: string[] } type MatchResult = { sourceId: string; searchTerm: string; confidence: 'exact' | 'likely' | 'low' | null; selectedUri: string | null; candidates: AlbumCandidate[]; trackMatches: Record } type PageItem = { source: ImportSourceRow; decision: { status: 'pending' | 'done' | 'skipped' | 'ignored-album' | 'ignored-artist'; excluded: boolean }; matchResult: MatchResult | null } -type PageView = { state: ImportStateView; artist: string; album: string; pageNumber: number; pageCount: number; rows: PageItem[]; options: { importContent: boolean; includeHistoricalPlayCounts: boolean; wholeAlbum: boolean; genre: string | null; rating: number | null; selectedTrackIds: string[] }; fuzzyGroups: Record; countModes: Record; lockedCountModes: string[] } +type PageView = { state: ImportStateView; batchId: number; artist: string; album: string; pageNumber: number; pageCount: number; rows: PageItem[]; options: { importContent: boolean; includeHistoricalPlayCounts: boolean; wholeAlbum: boolean; genre: string | null; rating: number | null; selectedTrackIds: string[] }; fuzzyGroups: Record; countModes: Record; lockedCountModes: string[] } type PickerKind = ImportPickerKind type PickerState = { kind: PickerKind; sourceId: string; query: string } type FuzzyProps = { fuzzy?: ImportSourceRow[]; fuzzyTarget?: string; fuzzyExpanded: boolean; fuzzyMode: CountMode; fuzzyLocked: boolean; onFuzzyMode: (mode: CountMode) => void; onFuzzyToggle: () => void } const emptyDefaults: LastFmImportDefaults = { importContent: true, includeHistoricalPlayCounts: true, wholeAlbum: false } -const emptyState: ImportStateView = { phase: null, username: null, spotifyAccountId: null, nextPage: 1, totalPages: null, totalScrobbles: 0, includedScrobbles: 0, matchedRows: 0, matchTotal: 0, defaults: emptyDefaults, remaining: 0, retryableError: null, searchTerms: true } +const emptyState: ImportStateView = { phase: null, username: null, spotifyAccountId: null, nextPage: 1, totalPages: null, downloadedPages: 0, totalScrobbles: 0, includedScrobbles: 0, defaults: emptyDefaults, remaining: 0, retryableError: null, searchTerms: true } +const importQueuePageLimit = 100 + +async function loadImportQueue(): Promise { + const items: ImportQueueItem[] = [] + let cursor = 0 + let total: number | undefined + while (true) { + const page = await invoke('lastfm_import_queue', { cursor, limit: importQueuePageLimit }) + if (page.cursor !== cursor || page.items.length > importQueuePageLimit || (total !== undefined && page.total !== total)) throw new Error('Last.fm import queue pagination is inconsistent.') + total ??= page.total + items.push(...page.items) + if (page.nextCursor === null) { + if (items.length !== page.total) throw new Error('Last.fm import queue pagination is incomplete.') + return items + } + if (!Number.isSafeInteger(page.nextCursor) || page.nextCursor <= cursor || page.nextCursor > page.total) throw new Error('Last.fm import queue pagination is invalid.') + cursor = page.nextCursor + } +} function reviewForPage(page: PageView): ReviewState { const rows = page.rows.map((item) => item.source) @@ -62,7 +80,7 @@ function pageOptions(review: ReviewState) { function pageWithQueuePosition(page: PageView | null, queue: ImportQueueItem[], sort: 'plays' | 'artist' | 'batch' | 'lastPlayed'): PageView | null { if (!page) return null const ordered = sortImportQueue(queue, sort) - const index = ordered.findIndex((item) => item.artist === page.artist && item.album === page.album) + const index = ordered.findIndex((item) => item.page === page.batchId) return { ...page, pageNumber: index + 1, pageCount: ordered.length } } @@ -104,49 +122,26 @@ function ImportIntentChecks({ defaults, disabled = false, onChange }: { defaults function DownloadPane({ state, defaults, busy, onDefaults, onStart }: { state: ImportStateView; defaults: LastFmImportDefaults; busy: boolean; onDefaults: (defaults: LastFmImportDefaults) => void; onStart: () => void }) { const total = state.totalPages ?? 0 - const downloaded = Math.max(0, state.nextPage - 1) + const downloaded = state.downloadedPages const percent = total ? Math.min(100, Math.round((downloaded / total) * 100)) : 0 const isSetup = state.phase === null const isSuspended = state.phase === 'suspended' - const action = downloadAction(state.phase, state.retryableError !== null) + const isAggregating = state.phase === 'aggregating' + const action = downloadAction(state.phase, state.retryableError) return

LAST.FM HISTORY

-

{isSetup ? 'Import your complete Last.fm history' : isSuspended ? 'Import suspended for account safety' : 'Downloading your Last.fm history'}

-

{isSetup ? 'Retune takes a fixed snapshot and saves it page by page. You can review every match before anything is applied.' : isSuspended ? 'Reconnect the saved Last.fm and Spotify accounts before resuming this session.' : `Page ${state.nextPage}${state.totalPages ? ` of ${state.totalPages}` : ''} · ${state.includedScrobbles.toLocaleString()} scrobbles saved so far`}

+

{isSetup ? 'Import your complete Last.fm history' : isSuspended ? 'Import suspended for account safety' : isAggregating ? 'Preparing your review queue' : 'Downloading your Last.fm history'}

+

{isSetup ? 'Retune takes a fixed snapshot and saves it page by page. You can review every match before anything is applied.' : isSuspended ? 'Reconnect the saved Last.fm account before resuming this session.' : isAggregating ? 'All source pages are downloaded. Retune is sorting and grouping them before review.' : `Page ${state.nextPage}${state.totalPages ? ` of ${state.totalPages}` : ''} · ${state.includedScrobbles.toLocaleString()} scrobbles saved so far`}

{!isSetup && !isSuspended && <>{total ? `${downloaded} of ${total} pages downloaded` : 'Discovering the history size…'}} -

You can leave this running — Retune keeps playing, and matching starts as soon as the history lands.

- {state.retryableError &&

{state.retryableError.message} {state.retryableError.retryable ? `Attempt ${state.retryableError.attempt}. Resume to retry.` : ''}

} +

You can leave this running — Retune keeps playing, and Spotify is contacted only when you open a review batch.

+ {state.retryableError &&

{state.retryableError.message} {state.retryableError.retryable ? `Attempt ${state.retryableError.attempt}. Retrying automatically while Retune is running.` : ''}

}
} -function MatchingPane({ state }: { state: ImportStateView }) { - const [busy, setBusy] = useState(false) - const [error, setError] = useState() - const percent = state.matchTotal ? Math.round((state.matchedRows / state.matchTotal) * 100) : 0 - const resume = async () => { - setBusy(true) - setError(undefined) - try { await invoke('start_lastfm_import', { defaults: state.defaults }) } catch (reason) { setError(String(reason)) } finally { setBusy(false) } - } - return
-
-

SPOTIFY MATCH

-

Matching your Last.fm history to Spotify

-

Retune searches sequentially and saves each result so you can leave and resume safely.

- - {state.matchedRows.toLocaleString()} of {state.matchTotal.toLocaleString()} source tracks matched -

You can leave this running — the review queue appears when matching is complete.

- {state.retryableError &&

{state.retryableError.message}

} - {error &&

{error}

} - -
-
-} - function MatchPickerDialog({ kind, query: initialQuery, candidates, selectedUri, busy, onCancel, onSearch, onChoose }: { kind: PickerKind; query: string; candidates: AlbumCandidate[]; selectedUri: string | null; busy: boolean; onCancel: () => void; onSearch: (query: string) => void; onChoose: (uri: string) => void }) { const [query, setQuery] = useState(initialQuery) const [choice, setChoice] = useState(selectedUri ?? '') @@ -197,7 +192,7 @@ function ImportPage({ page, showQueries, onRefresh, onNext, onPrevious, onError setReview(next) setBusy(true) try { - await invoke('lastfm_import_options', { artist: page.artist, album: page.album, options: pageOptions(next) }) + await invoke('lastfm_import_options', { batchId: page.batchId, artist: page.artist, album: page.album, options: pageOptions(next) }) if (refreshQueue) await onRefresh() } catch (error) { onError(error) } finally { setBusy(false) } } @@ -211,7 +206,7 @@ function ImportPage({ page, showQueries, onRefresh, onNext, onPrevious, onError const apply = async (advance: boolean) => { setBusy(true) try { - await invoke('lastfm_import_apply', { artist: page.artist, album: page.album, selectedIds: [...review.checked], options: pageOptions(review) }) + await invoke('lastfm_import_apply', { batchId: page.batchId, artist: page.artist, album: page.album, selectedIds: [...review.checked], options: pageOptions(review) }) const nextQueue = await onRefresh() if (advance) onNext(nextQueue) } catch (error) { onError(error) } finally { setBusy(false) } @@ -234,13 +229,13 @@ function ImportPage({ page, showQueries, onRefresh, onNext, onPrevious, onError const searchPicker = async (query: string) => { if (!picker) return const activePicker = picker - await run(activePicker.kind === 'album' ? 'lastfm_import_change_album' : 'lastfm_import_change_track', { id: activePicker.sourceId, query }) + await run(activePicker.kind === 'album' ? 'lastfm_import_change_album' : 'lastfm_import_change_track', { batchId: page.batchId, id: activePicker.sourceId, query }) setPicker((current) => current && current.kind === activePicker.kind && current.sourceId === activePicker.sourceId ? { ...current, query } : current) } const choosePicker = async (uri: string) => { if (!picker) return const activePicker = picker - await run('lastfm_import_select_match', { id: activePicker.sourceId, uri }) + await run('lastfm_import_select_match', { batchId: page.batchId, id: activePicker.sourceId, uri }) setPicker((current) => current && current.kind === activePicker.kind && current.sourceId === activePicker.sourceId ? null : current) } const intentChange = (key: 'importContent' | 'includeHistoricalPlayCounts', checked: boolean) => { @@ -264,12 +259,12 @@ function ImportPage({ page, showQueries, onRefresh, onNext, onPrevious, onError } } return
-

{page.artist}

{page.album || 'Singles'}

{page.rows.length} source tracks · {page.rows.reduce((total, item) => total + item.source.playCount, 0).toLocaleString()} plays

Page {page.pageNumber} of {page.pageCount}
+

{page.artist}

{page.album || 'Singles'}

{page.rows.length} source tracks · {page.rows.reduce((total, item) => total + item.source.playCount, 0).toLocaleString()} plays

Batch {page.pageNumber} of {page.pageCount}

WHAT I’M IMPORTING

{page.album || 'Singles'}{page.artist} · {page.rows.length} source tracks

SPOTIFY MATCH

{page.rows[0]?.matchResult?.candidates.find((candidate) => candidate.uri === page.rows[0]?.matchResult?.selectedUri)?.name ?? 'Choose a release'}{page.rows[0]?.matchResult?.confidence ? confidenceLabel(page.rows[0].matchResult.confidence) : 'No release selected'}
{review.wholeAlbum &&

Exclude removes only this Last.fm source row. A track inherently included by the whole album cannot be removed from Spotify here.

} -
{page.rows.map((item) => void persist(toggleImportRow(review, item.source.stableId), true)} onExclude={() => void run('lastfm_import_review', { id: item.source.stableId, action: item.decision.excluded ? 'undo-exclude' : 'exclude', artist: page.artist, album: page.album })} onChangeTrack={() => openTrackPicker(item.source.stableId)} {...fuzzy(item)} fuzzyExpanded={fuzzy(item).fuzzyExpanded ?? false} fuzzyMode={fuzzy(item).fuzzyMode ?? 'sum'} fuzzyLocked={fuzzy(item).fuzzyLocked ?? false} onFuzzyMode={fuzzy(item).onFuzzyMode ?? (() => {})} onFuzzyToggle={fuzzy(item).onFuzzyToggle ?? (() => {})} />)}
-
{selectedImportCount(review)} selected · {excludedImportCount(review)} excluded · {restPendingImportCount(review)} rest pending
+
{page.rows.map((item) => void persist(toggleImportRow(review, item.source.stableId), true)} onExclude={() => void run('lastfm_import_review', { batchId: page.batchId, id: item.source.stableId, action: item.decision.excluded ? 'undo-exclude' : 'exclude', artist: page.artist, album: page.album })} onChangeTrack={() => openTrackPicker(item.source.stableId)} {...fuzzy(item)} fuzzyExpanded={fuzzy(item).fuzzyExpanded ?? false} fuzzyMode={fuzzy(item).fuzzyMode ?? 'sum'} fuzzyLocked={fuzzy(item).fuzzyLocked ?? false} onFuzzyMode={fuzzy(item).onFuzzyMode ?? (() => {})} onFuzzyToggle={fuzzy(item).onFuzzyToggle ?? (() => {})} />)}
+
{selectedImportCount(review)} selected · {excludedImportCount(review)} excluded · {restPendingImportCount(review)} rest pending
{picker && pickerItem && setPicker(null)} onSearch={searchPicker} onChoose={choosePicker} />}
} @@ -282,27 +277,29 @@ export default function LastFmImporter() { const [selected, setSelected] = useState(null) const [page, setPage] = useState(null) const [busy, setBusy] = useState(false) + const [pageLoading, setPageLoading] = useState(false) const [error, setError] = useState() const [acceptAllOpen, setAcceptAllOpen] = useState(false) + const [acceptAllSummary, setAcceptAllSummary] = useState<{ albumEntities: number; trackEntities: number } | null>(null) const [pendingDefaults, setPendingDefaults] = useState(emptyDefaults) const pageRequestGeneration = useRef(0) + const acceptAllRunning = useRef(false) const orderedQueue = useMemo(() => sortImportQueue(queue, sort), [queue, sort]) - const selectedArtist = selected?.artist - const selectedAlbum = selected?.album + const selectedPage = selected?.page const refresh = useCallback(async (): Promise => { const requestGeneration = ++pageRequestGeneration.current try { - const [nextState, nextQueue] = await Promise.all([invoke('lastfm_import_state'), invoke('lastfm_import_queue')]) + const [nextState, nextQueue] = await Promise.all([invoke('lastfm_import_state'), loadImportQueue()]) setState(nextState) setShowQueries(nextState.searchTerms) setQueue(nextQueue) setPendingDefaults(nextState.defaults) - const current = selectedArtist && selectedAlbum ? nextQueue.find((item) => item.artist === selectedArtist && item.album === selectedAlbum) : undefined + const current = selectedPage ? nextQueue.find((item) => item.page === selectedPage) : undefined const firstRemaining = sortImportQueue(nextQueue, sort).find((item) => item.remaining) const target = current ?? ((nextState.phase === 'review' || nextState.phase === 'done') ? firstRemaining : undefined) if (target) { - if (target.artist !== selectedArtist || target.album !== selectedAlbum) setSelected(target) - await applyCurrentImportPageResponse(requestGeneration, () => pageRequestGeneration.current, invoke('lastfm_import_page', { artist: target.artist, album: target.album }), (nextPage) => setPage(pageWithQueuePosition(nextPage, nextQueue, sort))) + if (target.page !== selectedPage) setSelected(target) + await applyCurrentImportPageResponse(requestGeneration, () => pageRequestGeneration.current, invoke('lastfm_import_page', { batchId: target.page, artist: target.artist, album: target.album }), (nextPage) => setPage(pageWithQueuePosition(nextPage, nextQueue, sort))) } else if (isCurrentImportPageResponse(requestGeneration, pageRequestGeneration.current) && nextState.phase !== 'review' && nextState.phase !== 'done') { setSelected(null) setPage(null) @@ -312,10 +309,10 @@ export default function LastFmImporter() { if (isCurrentImportPageResponse(requestGeneration, pageRequestGeneration.current)) setError(String(reason)) return [] } - }, [selectedArtist, selectedAlbum, sort]) + }, [selectedPage, sort]) useEffect(() => { void refresh() - const subscription = listen('lastfm-import-changed', () => { void refresh() }) + const subscription = listen('lastfm-import-changed', () => { if (!acceptAllRunning.current) void refresh() }) return () => { void subscription.then((stop) => stop()) } }, [refresh]) useEffect(() => { setPage((current) => pageWithQueuePosition(current, queue, sort)) }, [queue, sort]) @@ -338,10 +335,13 @@ export default function LastFmImporter() { } const openQueueItem = async (item: ImportQueueItem, queueSnapshot = queue) => { const requestGeneration = pageRequestGeneration.current + 1 + setPageLoading(true) try { - await loadSelectedImportPage(pageRequestGeneration, item, (target) => invoke('lastfm_import_page', { artist: target.artist, album: target.album }), setSelected, (nextPage) => setPage(pageWithQueuePosition(nextPage, queueSnapshot, sort))) + await loadSelectedImportPage(pageRequestGeneration, item, (target) => invoke('lastfm_import_page', { batchId: target.page, artist: target.artist, album: target.album }), setSelected, (nextPage) => setPage(pageWithQueuePosition(nextPage, queueSnapshot, sort)), () => setPage(null)) } catch (reason) { if (isCurrentImportPageResponse(requestGeneration, pageRequestGeneration.current)) setError(String(reason)) + } finally { + setPageLoading(false) } } const nextQueueItem = (queueSnapshot = queue) => { @@ -350,16 +350,27 @@ export default function LastFmImporter() { else { setSelected(null); setPage(null); void refresh() } } const previousQueueItem = () => { - const index = selected ? orderedQueue.findIndex((item) => item.artist === selected.artist && item.album === selected.album) : orderedQueue.length + const index = selected ? orderedQueue.findIndex((item) => item.page === selected.page) : orderedQueue.length const previous = orderedQueue.slice(0, index).reverse().find((item) => item.remaining) ?? orderedQueue.slice(index + 1).reverse().find((item) => item.remaining) if (previous) void openQueueItem(previous) } const acceptAll = async () => { - setBusy(true); setError(undefined) + setBusy(true); setError(undefined); acceptAllRunning.current = true try { - for (const item of orderedQueue.filter((entry) => entry.remaining)) { await invoke('lastfm_import_accept_all_page', { artist: item.artist, album: item.album }); await refresh() } + for (const item of orderedQueue.filter((entry) => entry.remaining)) await invoke('lastfm_import_accept_all_page', { batchId: item.page, artist: item.artist, album: item.album }) + await refresh() setAcceptAllOpen(false) - } catch (reason) { setError(String(reason)) } finally { setBusy(false) } + setAcceptAllSummary(null) + } catch (reason) { setError(String(reason)) } finally { acceptAllRunning.current = false; setBusy(false) } + } + const prepareAcceptAll = async () => { + setBusy(true); setError(undefined); acceptAllRunning.current = true + try { + const summary = await invoke<{ albumEntities: number; trackEntities: number }>('lastfm_import_prepare_accept_all') + await refresh() + setAcceptAllSummary(summary) + setAcceptAllOpen(true) + } catch (reason) { setError(String(reason)) } finally { acceptAllRunning.current = false; setBusy(false) } } const setSearchTerms = async (show: boolean) => { setShowQueries(show) @@ -367,13 +378,12 @@ export default function LastFmImporter() { try { await invoke('lastfm_import_search_terms', { show }) } catch (reason) { setError(String(reason)) } finally { setBusy(false) } } const reviewReady = state.phase === 'review' || state.phase === 'done' - const albumEntities = orderedQueue.filter((item) => item.remaining).reduce((total, item) => total + item.albumEntities, 0) - const trackEntities = orderedQueue.filter((item) => item.remaining).reduce((total, item) => total + item.trackEntities, 0) + const emptyPage = importEmptyPageMessage(state.phase, pageLoading) return
-

LAST.FM HISTORY

Last.fm importer

{importStatusText(state.phase, state.username, state.nextPage, state.totalPages, state.matchedRows, state.matchTotal)}{showsImportRemaining(state.phase) && state.remaining ? ` · ${state.remaining.toLocaleString()} left` : ''}

Powered by Last.fm{reviewReady && <>Sort
{([['plays', 'Most played'], ['artist', 'Artist A–Z'], ['batch', 'Batch size'], ['lastPlayed', 'Last played']] as const).map(([value, label]) => )}
}
+

LAST.FM HISTORY

Last.fm importer

{importStatusText(state.phase, state.username, state.nextPage, state.totalPages)}{showsImportRemaining(state.phase) && state.remaining ? ` · ${state.remaining.toLocaleString()} left` : ''}

Powered by Last.fm{reviewReady && <>Sort
{([['plays', 'Most played'], ['artist', 'Artist A–Z'], ['batch', 'Batch size'], ['lastPlayed', 'Last played']] as const).map(([value, label]) => )}
}
{error &&
{error}
} - {state.phase === 'downloading' || state.phase === null || state.phase === 'suspended' ? void start()} /> : state.phase === 'matching' ? :
{page ? setError(String(reason))} /> :
No review page selectedSelect an album from the queue.
}
} + {state.phase === 'downloading' || state.phase === 'aggregating' || state.phase === null || state.phase === 'suspended' ? void start()} /> :
{page ? setError(String(reason))} /> :
{emptyPage.title}{emptyPage.detail}
}
}
Last.fm is an absolute historical baseline. Existing Retune plays are never erased.{state.username ? `Last.fm: ${state.username}` : 'Account not connected'}
- {acceptAllOpen && setAcceptAllOpen(false)} onConfirm={() => void acceptAll()} />} + {acceptAllOpen && acceptAllSummary && { setAcceptAllOpen(false); setAcceptAllSummary(null) }} onConfirm={() => void acceptAll()} />}
} diff --git a/apps/desktop/src/appState.ts b/apps/desktop/src/appState.ts index c96eb1d..53747c5 100644 --- a/apps/desktop/src/appState.ts +++ b/apps/desktop/src/appState.ts @@ -107,6 +107,7 @@ export const defaultSettings: Settings = { gapless: true, playThresholdPercent: 100, lastfmScrobbling: true, + lastfmScrobblingProfile: null, } export const initialState: State = { @@ -128,7 +129,7 @@ export const initialState: State = { playbackAuthorization: null, connection: { connected: false, needs_reauth: false, playback_authorized: false }, lastfm: { available: false, connected: false, username: null, pending: false, reconnectRequired: false, problem: null }, - lastfmImport: { phase: null, username: null, spotifyAccountId: null, nextPage: 1, totalPages: null, totalScrobbles: 0, includedScrobbles: 0, matchedRows: 0, matchTotal: 0, defaults: { importContent: true, includeHistoricalPlayCounts: true, wholeAlbum: false }, remaining: 0, retryableError: null, searchTerms: true }, + lastfmImport: { phase: null, username: null, spotifyAccountId: null, nextPage: 1, totalPages: null, downloadedPages: 0, totalScrobbles: 0, includedScrobbles: 0, defaults: { importContent: true, includeHistoricalPlayCounts: true, wholeAlbum: false }, remaining: 0, retryableError: null, searchTerms: true }, spotifyResults: null, spotifySearching: false, playlistRevision: 0, diff --git a/apps/desktop/src/lastfmImportState.ts b/apps/desktop/src/lastfmImportState.ts index 26edc58..448152d 100644 --- a/apps/desktop/src/lastfmImportState.ts +++ b/apps/desktop/src/lastfmImportState.ts @@ -1,5 +1,5 @@ export type ImportSort = 'plays' | 'artist' | 'batch' | 'lastPlayed' -export type ImportPhase = 'downloading' | 'matching' | 'review' | 'done' | 'suspended' +export type ImportPhase = 'downloading' | 'aggregating' | 'review' | 'done' | 'suspended' export type CountMode = 'sum' | 'overwrite' | 'zero' export type ImportPickerKind = 'album' | 'track' export type ImportConfidence = 'exact' | 'likely' | 'low' | null @@ -17,17 +17,25 @@ export type ImportVariant = { } export type ImportQueueItem = { + page: number artist: string album: string playCount: number latest: number - sourceIds: string[] + sourceCount: number remaining: boolean albumEntities: number trackEntities: number status?: QueueStatus | null } +export type ImportQueuePage = { + items: ImportQueueItem[] + cursor: number + nextCursor: number | null + total: number +} + export type ImportSourceRow = { stableId: string artist: string @@ -185,16 +193,16 @@ export function resolveImportCount(rows: ImportSourceRow[], mode: CountMode): nu export function sortImportQueue(items: ImportQueueItem[], sort: ImportSort): ImportQueueItem[] { return [...items].sort((left, right) => { - if (sort === 'artist') return left.artist.localeCompare(right.artist) || left.album.localeCompare(right.album) - if (sort === 'batch') return right.sourceIds.length - left.sourceIds.length || left.artist.localeCompare(right.artist) - if (sort === 'lastPlayed') return right.latest - left.latest || left.artist.localeCompare(right.artist) - return right.playCount - left.playCount || left.artist.localeCompare(right.artist) + if (sort === 'artist') return left.artist.localeCompare(right.artist) || left.album.localeCompare(right.album) || left.page - right.page + if (sort === 'batch') return right.sourceCount - left.sourceCount || left.artist.localeCompare(right.artist) || left.page - right.page + if (sort === 'lastPlayed') return right.latest - left.latest || left.artist.localeCompare(right.artist) || left.page - right.page + return right.playCount - left.playCount || left.artist.localeCompare(right.artist) || left.page - right.page }) } export function nextRemainingImportQueue(items: ImportQueueItem[], current: ImportQueueItem | null, sort: ImportSort): ImportQueueItem | null { const ordered = sortImportQueue(items, sort) - const currentIndex = current ? ordered.findIndex((item) => item.artist === current.artist && item.album === current.album) : -1 + const currentIndex = current ? ordered.findIndex((item) => item.page === current.page) : -1 return ordered.slice(currentIndex + 1).find((item) => item.remaining) ?? ordered.slice(0, Math.max(0, currentIndex)).find((item) => item.remaining) ?? null } @@ -202,25 +210,33 @@ export function importStatusLabel(status: QueueStatus): string { return status === 'ignored-album' ? 'ignored-album' : status === 'ignored-artist' ? 'ignored-artist' : status } -export function importStatusText(phase: ImportPhase | null, username: string | null, nextPage: number, totalPages: number | null, matchedRows: number, matchTotal: number): string { +export function importStatusText(phase: ImportPhase | null, username: string | null, nextPage: number, totalPages: number | null): string { if (phase === 'downloading') return `Downloading Last.fm history · page ${nextPage}${totalPages ? ` of ${totalPages}` : ''}` - if (phase === 'matching') return `Matching Last.fm history · ${matchedRows.toLocaleString()} of ${matchTotal.toLocaleString()} tracks` + if (phase === 'aggregating') return 'Preparing Last.fm review' if (phase === 'suspended') return 'Import suspended for account safety' if (phase === 'done') return 'Import complete' - return username ? 'Ready to import' : 'Connect Last.fm and Spotify to begin' + return username ? 'Ready to import' : 'Connect Last.fm to begin' } export function showsImportRemaining(phase: ImportPhase | null): boolean { return phase === 'review' || phase === 'done' } -export function downloadAction(phase: ImportPhase | null, hasRetryableError: boolean): { label: string; disabled: boolean } { +export function downloadAction(phase: ImportPhase | null, retryableError: { retryable: boolean } | null): { label: string; disabled: boolean } { if (phase === null) return { label: 'Start import', disabled: false } if (phase === 'suspended') return { label: 'Check accounts and resume', disabled: false } - if (phase === 'downloading' && !hasRetryableError) return { label: 'Downloading…', disabled: true } + if (retryableError?.retryable && (phase === 'downloading' || phase === 'aggregating')) return { label: 'Retrying automatically…', disabled: true } + if (phase === 'aggregating' && !retryableError) return { label: 'Preparing review…', disabled: true } + if (phase === 'downloading' && !retryableError) return { label: 'Downloading…', disabled: true } return { label: 'Resume download', disabled: false } } +export function importEmptyPageMessage(phase: ImportPhase | null, pageLoading: boolean): { title: string; detail: string } { + if (pageLoading) return { title: 'Matching this review batch…', detail: 'Spotify is contacted only for the visible batch.' } + if (phase === 'done') return { title: 'Import complete', detail: 'All review batches are complete.' } + return { title: 'No review page selected', detail: 'Select an album from the queue.' } +} + export function isCurrentImportPageResponse(requestGeneration: number, currentGeneration: number): boolean { return requestGeneration === currentGeneration } @@ -230,8 +246,9 @@ export async function applyCurrentImportPageResponse(requestGeneration: numbe if (isCurrentImportPageResponse(requestGeneration, currentGeneration())) apply(value) } -export async function loadSelectedImportPage(generation: { current: number }, item: ImportQueueItem, load: (item: ImportQueueItem) => Promise, select: (item: ImportQueueItem) => void, apply: (value: T) => void): Promise { +export async function loadSelectedImportPage(generation: { current: number }, item: ImportQueueItem, load: (item: ImportQueueItem) => Promise, select: (item: ImportQueueItem) => void, apply: (value: T) => void, invalidate: () => void = () => {}): Promise { const requestGeneration = ++generation.current + invalidate() select(item) await applyCurrentImportPageResponse(requestGeneration, () => generation.current, load(item), apply) } diff --git a/apps/desktop/src/types.ts b/apps/desktop/src/types.ts index 3b067fa..617909b 100644 --- a/apps/desktop/src/types.ts +++ b/apps/desktop/src/types.ts @@ -36,6 +36,7 @@ export type Settings = { gapless: boolean playThresholdPercent: PlayThresholdPercent lastfmScrobbling: boolean + lastfmScrobblingProfile: { username: string; startedAt: number } | null } export type ConnectionState = { connected: boolean; needs_reauth: boolean; playback_authorized: boolean } @@ -53,15 +54,14 @@ export type LastFmImportDefaults = { wholeAlbum: boolean } export type LastFmImportState = { - phase: 'downloading' | 'matching' | 'review' | 'done' | 'suspended' | null + phase: 'downloading' | 'aggregating' | 'review' | 'done' | 'suspended' | null username: string | null spotifyAccountId: string | null nextPage: number totalPages: number | null + downloadedPages: number totalScrobbles: number includedScrobbles: number - matchedRows: number - matchTotal: number defaults: LastFmImportDefaults remaining: number retryableError: { message: string; attempt: number; retryable: boolean } | null diff --git a/apps/desktop/test/ui.test.ts b/apps/desktop/test/ui.test.ts index 5a9ba18..3fdbb20 100644 --- a/apps/desktop/test/ui.test.ts +++ b/apps/desktop/test/ui.test.ts @@ -4,7 +4,7 @@ import { formatDiagnosticReport, reportWindow, type DiagnosticEntry } from '../s import { initialState, reducer, type Action } from '../src/appState.ts' import type { BrowseView, PlaybackTrack, Selection, Settings, SpotifyResults } from '../src/types.ts' import { createSpotifySearchState, expandSpotifySearchGroup, failSpotifySearchGroup, moreSpotifySearchLabel, receiveSpotifySearchPage, replaceSpotifySearchResults, resetSpotifySearchQuery, retrySpotifySearchGroup, setSpotifySearchTab, spotifyMembership, spotifySearchGroupHeader, spotifySearchPendingPageKey } from '../src/spotifySearch.ts' -import { acceptImportAndNext, acceptImportChanges, defaultReviewState, downloadAction, excludedImportCount, excludeImportRow, ignoreImportAlbum, ignoreImportArtist, importStatusText, isCurrentImportPageResponse, loadSelectedImportPage, nextRemainingImportQueue, pickerCandidates, pickerSelectedUri, remainingImportCount, resolveImportCount, restPendingImportCount, selectedImportCount, selectedImportTrackConfidence, showsImportRemaining, skipImportAlbum, sortImportQueue, toggleImportRow, trackPickerQuery, validImportIntent, type ImportQueueItem, type ImportSourceRow } from '../src/lastfmImportState.ts' +import { acceptImportAndNext, acceptImportChanges, defaultReviewState, downloadAction, excludedImportCount, excludeImportRow, ignoreImportAlbum, ignoreImportArtist, importEmptyPageMessage, importStatusText, isCurrentImportPageResponse, loadSelectedImportPage, nextRemainingImportQueue, pickerCandidates, pickerSelectedUri, remainingImportCount, resolveImportCount, restPendingImportCount, selectedImportCount, selectedImportTrackConfidence, showsImportRemaining, skipImportAlbum, sortImportQueue, toggleImportRow, trackPickerQuery, validImportIntent, type ImportQueueItem, type ImportSourceRow } from '../src/lastfmImportState.ts' import { appliedZoom, browseRequestKey, browseViewForRequest, clearedTrackRating, compareTracks, contiguousRange, dialogTabTarget, facetLabel, insertionIndexAtY, isCurrentTrack, LIBRARY_DEFAULT_COLUMN_ORDER, LIBRARY_DEFAULT_HIDDEN_COLUMNS, menuPosition, mergeByUri, moveBefore, moveToIndex, nextNativeDragActive, normalizeZoom, overlayEditTargets, pendingPlaybackTarget, playbackAuthorizationPrompt, playbackOriginAction, playbackQueue, playbackRetryReady, playbackStartAction, playlistLayoutFor, playlistOverride, playlistRows, PLAYLIST_DEFAULT_COLUMN_ORDER, PLAYLIST_DEFAULT_HIDDEN_COLUMNS, rememberSelection, restoreSelection, resizedColumnWidth, resizedPaneHeight, selectionAfterFacet, staleSelectionFacet, SYNTHETIC_BASE, visibleColumnOrder } from '../src/ui.ts' const searchPage = (overrides: Partial = {}): SpotifyResults => ({ @@ -20,8 +20,8 @@ const importRows = (): ImportSourceRow[] => [ ] const importQueue = (): ImportQueueItem[] => [ - { artist: 'Beta', album: 'Album', playCount: 3, latest: 30, sourceIds: ['a'], remaining: true, albumEntities: 1, trackEntities: 0 }, - { artist: 'Alpha', album: 'Album', playCount: 2, latest: 40, sourceIds: ['b', 'c'], remaining: true, albumEntities: 0, trackEntities: 2 }, + { page: 1, artist: 'Beta', album: 'Album', playCount: 3, latest: 30, sourceCount: 1, remaining: true, albumEntities: 1, trackEntities: 0 }, + { page: 2, artist: 'Alpha', album: 'Album', playCount: 2, latest: 40, sourceCount: 2, remaining: true, albumEntities: 0, trackEntities: 2 }, ] test('diagnostic reports include session context through the last problem only', () => { @@ -103,27 +103,37 @@ test('Last.fm A-to-B queue selection keeps the newer page when A resolves last', const queue = importQueue() const selected: string[] = [] const applied: string[] = [] - const requestA = loadSelectedImportPage(generation, queue[0], () => responseA, (item) => selected.push(item.artist), (page) => applied.push(page)) - const requestB = loadSelectedImportPage(generation, queue[1], () => responseB, (item) => selected.push(item.artist), (page) => applied.push(page)) + let visible: string | null = 'A' + const requestA = loadSelectedImportPage(generation, queue[0], () => responseA, (item) => selected.push(item.artist), (page) => { visible = page; applied.push(page) }, () => { visible = null }) + const requestB = loadSelectedImportPage(generation, queue[1], () => responseB, (item) => selected.push(item.artist), (page) => { visible = page; applied.push(page) }, () => { visible = null }) + assert.equal(visible, null) resolveB('B') await requestB + assert.equal(visible, 'B') resolveA('A') await requestA assert.deepEqual(selected, ['Beta', 'Alpha']) assert.deepEqual(applied, ['B']) + assert.equal(visible, 'B') }) test('Last.fm setup and download status reflect connected and active states', () => { - assert.equal(importStatusText(null, 'rianjs', 1, null, 0, 0), 'Ready to import') - assert.equal(importStatusText(null, null, 1, null, 0, 0), 'Connect Last.fm and Spotify to begin') - assert.equal(importStatusText('downloading', 'rianjs', 7, 1230, 0, 0), 'Downloading Last.fm history · page 7 of 1230') + assert.equal(importStatusText(null, 'rianjs', 1, null), 'Ready to import') + assert.equal(importStatusText(null, null, 1, null), 'Connect Last.fm to begin') + assert.equal(importStatusText('downloading', 'rianjs', 7, 1230), 'Downloading Last.fm history · page 7 of 1230') assert.equal(showsImportRemaining('downloading'), false) - assert.equal(showsImportRemaining('matching'), false) + assert.equal(showsImportRemaining('aggregating'), false) assert.equal(showsImportRemaining('review'), true) - assert.deepEqual(downloadAction(null, false), { label: 'Start import', disabled: false }) - assert.deepEqual(downloadAction('downloading', false), { label: 'Downloading…', disabled: true }) - assert.deepEqual(downloadAction('downloading', true), { label: 'Resume download', disabled: false }) - assert.deepEqual(downloadAction('suspended', false), { label: 'Check accounts and resume', disabled: false }) + assert.deepEqual(downloadAction(null, null), { label: 'Start import', disabled: false }) + assert.deepEqual(downloadAction('downloading', null), { label: 'Downloading…', disabled: true }) + assert.deepEqual(downloadAction('downloading', { retryable: true }), { label: 'Retrying automatically…', disabled: true }) + assert.deepEqual(downloadAction('downloading', { retryable: false }), { label: 'Resume download', disabled: false }) + assert.deepEqual(downloadAction('suspended', null), { label: 'Check accounts and resume', disabled: false }) + assert.deepEqual(downloadAction('aggregating', null), { label: 'Preparing review…', disabled: true }) + assert.deepEqual(downloadAction('aggregating', { retryable: false }), { label: 'Resume download', disabled: false }) + assert.deepEqual(importEmptyPageMessage('review', true), { title: 'Matching this review batch…', detail: 'Spotify is contacted only for the visible batch.' }) + assert.deepEqual(importEmptyPageMessage('review', false), { title: 'No review page selected', detail: 'Select an album from the queue.' }) + assert.deepEqual(importEmptyPageMessage('done', false), { title: 'Import complete', detail: 'All review batches are complete.' }) }) test('Last.fm track picker starts from the source row and cancel preserves album and row matches', () => { diff --git a/docs/architecture/library.md b/docs/architecture/library.md index 4c89b2d..daef5c9 100644 --- a/docs/architecture/library.md +++ b/docs/architecture/library.md @@ -50,12 +50,43 @@ plays or `last_played_at`; counts-only performs no Spotify write and updates only already-materialized matched Retune tracks. The importer’s “Show Spotify search terms” preference is session-level and is -restored on resume. Fuzzy disclosures use all source rows in the session that -resolve to a target, so the target-wide count decision remains truthful when -those rows came from different artist/album pages. - -The source snapshot stores compact spelling variants with count, earliest, and -latest timestamps rather than raw Last.fm responses. Excluded rows remain +restored on resume. Fuzzy disclosures are bounded to the visible persisted +`ImportBatch`; the target-wide count decision still includes completed source +rows from other batches. + +The source importer is V2. It records a fixed profile-bound `historyTo`, probes +Last.fm metadata once, downloads pages at the documented 200-row limit from +oldest toward page 1, and stores parsed raw pages under a snapshot-specific +machine cache. Rows at or after `historyTo` are rejected before caching or +counting; the exact Last.fm username is recorded in the manifest and each page. +The manifest is authoritative: an orphan page file is harmless and may be +overwritten, while an acknowledged missing, corrupt, oversized, or +metadata-mismatched page quarantines the whole snapshot and restarts V2. A +retryable Last.fm failure is persisted and retried in-process at the capped +backoff without advancing the cursor. No aggregation happens until every page +is acknowledged; then raw-page reads, sorting, and aggregation run off the +async runtime before review is entered atomically (or Done when no rows remain) +and the cache is best-effort deleted. A saved Downloading or Aggregating session +is claimed and resumed once by the Tauri shell after Last.fm hydration using its +stored username and cutoff; Downloading remains Last.fm-only, while Aggregating +also verifies the live Last.fm username before claiming the runner and suspends +with redacted state on mismatch. The owner is revalidated and held through the +aggregation transition and emitted state, so a connected-account change cannot +expose the completed snapshot. React only observes that work. An empty state +does not create one. + +Review batches are stable 1-based `ImportBatch` pages capped at 100 source rows. +Normal albums under the cap remain one batch; larger albums and singles split +deterministically, and command arguments must identify the requested batch and +its source rows. Queue summaries cross the IPC boundary as bounded cursor/limit +pages with `sourceCount`, not source IDs; Accept All applies its prepared batches +sequentially and refreshes the queue once after the bulk operation. + +Spotify matching is lazy. Opening a visible batch uses the shared client and +request gate, serializes duplicate requests, binds the Spotify account on the +first match, and caches the result; reopening it makes no API call. Accept All +is the only bulk exception and prepares remaining batches sequentially before +showing global unique album/track URI counts and awaiting confirmation. Excluded rows remain source-history decisions and can be undone before acceptance; they never remove a track inherently materialized by a saved whole album. diff --git a/docs/architecture/persistence.md b/docs/architecture/persistence.md index d9f37ec..530f246 100644 --- a/docs/architecture/persistence.md +++ b/docs/architecture/persistence.md @@ -18,7 +18,8 @@ All JSON state writes use a temporary file followed by atomic rename. | `dev-lastfm-session.json` | Development Last.fm session; mode 0600 on Unix | | `lastfm-pending-token.json` | Short-lived Last.fm authorization token; mode 0600 on Unix | | `lastfm-scrobbles.json` | Ordered durable Last.fm scrobble queue; excluded from backup | -| `lastfm-import.json` | Versioned, account-bound Last.fm snapshot/match/review session; excluded from backup | +| `lastfm-import.json` | Versioned, account-bound Last.fm snapshot/review session; excluded from backup | +| `lastfm-import-cache/` | Disposable V2 parsed-page cache and authoritative manifests; excluded from backup | The official Tauri window-state plugin manages the main native window's size, position, and maximized state in machine-local application state. Its lifecycle @@ -59,23 +60,46 @@ with the same temporary-file-and-rename atomic replacement as other app data. Missing or incomplete state is unknown and does not authorize destructive reconciliation until a complete sync establishes the exact account state. -`lastfm-import.json` is `LastFmImportSessionV1`. It stores the fixed snapshot -end timestamp, Last.fm username, Spotify `/me` account ID, phase/page cursor, -totals, retryable error and attempt, session defaults for the two independent -intents (content and historical play counts) plus whole-album mode, compact page -checkpoints with unique source IDs, spelling variants, match -results/candidates/selected URIs, decisions, page options, a session-level -Spotify-target-to-Sum/Overwrite/Zero map, and the session-level search-term -display preference. A response page must match the requested -cursor before the atomic checkpoint advances. It is written with the Last.fm -atomic replacement helper and mode 0600 on Unix. Serialized sessions are capped -at 100 MiB; corrupt or unknown versions are quarantined and never applied. This -machine/account state is deliberately outside normal backup/restore, like -`spotify-library.json` and the scrobble queue. +`lastfm-import.json` is `LastFmImportSessionV2`. It stores the immutable +`historyTo`, Last.fm username, nullable Spotify `/me` account ID, snapshot cache +ID, descending page cursor, downloaded/total pages, totals, retryable error and +attempt, session defaults for the two independent intents (content and +historical play counts) plus whole-album mode, compact aggregated rows, +decisions, stable 1-based `ImportBatch` pages capped at 100 source rows, batch +options, match results/candidates/selected URIs, a +session-level Spotify-target-to-Sum/Overwrite/Zero map, and the session-level +search-term display preference. Last.fm pages are written atomically as parsed +raw-page files; the manifest is written only after the page file and is the +authority for recovery. The manifest and every page record the exact Last.fm +username as well as cutoff/page metadata, so punctuation-distinct accounts +cannot share a snapshot. Unacknowledged orphan files are ignored/overwritten. +An acknowledged missing, corrupt, oversized, or metadata-mismatched page +quarantines the entire snapshot and starts a fresh V2 session. The source +runner retries the same probe/page after Last.fm's capped internal retry is +exhausted, persisting state and waiting at the capped delay without advancing +the cursor. No raw page is aggregated until the manifest is complete; review +entry and cache cleanup follow one atomic session write. Session and cache +files use mode 0600 on Unix and a 100 MiB safety ceiling. Corrupt or unknown +session versions are quarantined and never applied. This machine/account state +is deliberately outside normal backup/restore, like `spotify-library.json` and +the scrobble queue. + +`lastfmScrobblingProfile` is persisted in settings and is accepted only when +its trimmed username is non-empty and `startedAt` is positive. Settings load, +save, and export restore all validate this boundary. + +`settings.json` carries the exportable optional +`lastfmScrobblingProfile` (`username`, `startedAt`). Missing legacy profiles +are backfilled on the first successful enable/import; the same username keeps +its cutoff across toggles, while a different username replaces it. A successful +live validation keeps the completed V2 session, profile, and recovery backup; +the backup is restored only when validation fails or rollback is required. Every session read-modify-write is serialized from its in-memory snapshot through JSON serialization, blocking atomic replacement, and the in-memory -swap; suspended account-bound reads are redacted rather than exposing the -previous owner. +swap. Raw-page writes and aggregation are kept off the async runtime, and the +session cursor is rechecked after a page write before acknowledgement. +Suspended account-bound reads are redacted rather than exposing the previous +owner. Column layout is UI state in `settings.json`: the Library has one order, width map, and hidden-column list. Playlists have independent metadata-column order, width, diff --git a/docs/architecture/spotify.md b/docs/architecture/spotify.md index 231063c..82b9efe 100644 --- a/docs/architecture/spotify.md +++ b/docs/architecture/spotify.md @@ -117,11 +117,22 @@ filtered group starts at ten. Query changes discard pages; filter changes reset visible counts but retain pages for the same query. A failed later page leaves existing rows visible and can be retried for that group. -Last.fm album matching reuses the same client and request gate with official -`album:`/`artist:` field filters and a limit of 10, then fetches each candidate's -tracks for set-overlap classification. Matching does not hold the membership -mutex across the history; only account checks and the final content save hold -that gate. +Last.fm source download and aggregation do not use Spotify or its account +gate. Opening a visible review batch lazily matches it through this same shared +client/request gate with official `album:`/`artist:` field filters and a limit +of 10, then fetches candidate tracks for set-overlap classification. An +importer-wide async lock serializes duplicate batch matches; cached revisits +make no matching/search request and there is no adjacent prefetch. A cached +Spotify-derived page trusts only an exact cached library identity; an inexact +identity resolves Spotify `/me` before the page is exposed. The first +successful match binds the session to Spotify `/me`; its final ownership check +and durable match mutation stay under the shared membership gate, and a later +identity mismatch suspends Spotify-derived work. Accept All is the explicit +sequential bulk exception: it sequentially prepares every remaining batch, +reports global unique album/track URI counts, and only then permits +confirmation and application. Review batches are stable persisted pages capped +at 100 source rows, so matching, fuzzy disclosures, and command source-ID +validation never widen to an adjacent batch. ## Writes and playlists @@ -137,14 +148,15 @@ detect concurrent changes, then reloads stale state. Retune does not request or mutate item contents for playlists the current user does not own; it may display their available metadata and cached counts. -The Last.fm importer reuses the same membership gate and shared client. Its -album search uses the official `album:`/`artist:` field filters with a limit of -10, then fetches candidate album tracks for overlap classification; track -rematching uses a direct `track:` search. Import album acceptance calls the -same reusable album operation as the main UI and sends one album URI. Import -track acceptance calls the reusable track operation and sends only selected -track URIs. The generic `PUT /me/library` path is used; deprecated timestamped -track-save endpoints are not used. +The Last.fm importer reuses this shared client only for visible-batch matching +and explicit content acceptance. Its album search uses the official +`album:`/`artist:` field filters with a limit of 10, then fetches candidate album +tracks for overlap classification; track rematching uses a direct `track:` +search. Import album acceptance calls the same reusable album operation as the +main UI and sends one album URI. Import track acceptance calls the reusable +track operation and sends only selected track URIs. The generic +`PUT /me/library` path is used; deprecated timestamped track-save endpoints are +not used. ## Contract changes