diff --git a/backend/database/sql/queries/audio_files.sql b/backend/database/sql/queries/audio_files.sql index 2bb789c..4bbfc78 100644 --- a/backend/database/sql/queries/audio_files.sql +++ b/backend/database/sql/queries/audio_files.sql @@ -343,3 +343,7 @@ JOIN audio_files af ON af.recording_id = r.id WHERE r.mbid IN (sqlc.slice('mbids')) AND af.library_id = ? ORDER BY af.file_path; + +-- name: GetAudioFilesByPaths :many +SELECT id, library_id, file_path, group_key FROM audio_files +WHERE file_path IN (sqlc.slice('paths')); diff --git a/backend/database/sql/queries/excluded_paths.sql b/backend/database/sql/queries/excluded_paths.sql new file mode 100644 index 0000000..45c3060 --- /dev/null +++ b/backend/database/sql/queries/excluded_paths.sql @@ -0,0 +1,15 @@ +-- name: ExcludePath :exec +INSERT INTO excluded_paths (library_id, file_path) +VALUES (?, ?) +ON CONFLICT(library_id, file_path) DO NOTHING; + +-- name: GetExcludedPathsByLibrary :many +SELECT file_path FROM excluded_paths +WHERE library_id = ?; + +-- name: CountExcludedPathsByLibrary :one +SELECT COUNT(*) FROM excluded_paths +WHERE library_id = ?; + +-- name: ClearExcludedPaths :exec +DELETE FROM excluded_paths; diff --git a/backend/database/sql/schemas/excluded_paths.sql b/backend/database/sql/schemas/excluded_paths.sql new file mode 100644 index 0000000..27dc719 --- /dev/null +++ b/backend/database/sql/schemas/excluded_paths.sql @@ -0,0 +1,25 @@ +-- Paths the user has removed from the library, which the scanner must +-- not import again. +-- +-- "Remove from library" deletes the audio_files row and leaves the file +-- on disk. Without this table the next scan finds the file, sees no +-- row for it, and imports it again — so the exclusion is not an +-- enhancement, it is what makes the operation mean anything. +-- +-- A row is keyed by (library_id, file_path) rather than by audio_file +-- id, because the row it names has just been deleted. ON DELETE +-- CASCADE from libraries means removing a library takes its exclusions +-- with it; a full rescan clears the table outright, which is the only +-- way back for a path removed by mistake until there is a UI for it. + +CREATE TABLE IF NOT EXISTS excluded_paths ( + id INTEGER PRIMARY KEY, + library_id INTEGER NOT NULL, + file_path TEXT NOT NULL, + excluded_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + UNIQUE(library_id, file_path), + FOREIGN KEY(library_id) REFERENCES libraries(id) ON DELETE CASCADE +); + +CREATE INDEX IF NOT EXISTS idx_excluded_paths_library + ON excluded_paths(library_id); diff --git a/backend/database/sql/sqlcgen/audio_files.sql.go b/backend/database/sql/sqlcgen/audio_files.sql.go index 7062065..6ca2729 100644 --- a/backend/database/sql/sqlcgen/audio_files.sql.go +++ b/backend/database/sql/sqlcgen/audio_files.sql.go @@ -648,6 +648,56 @@ func (q *Queries) GetAudioFilesByLibrary(ctx context.Context, libraryID int64) ( return items, nil } +const getAudioFilesByPaths = `-- name: GetAudioFilesByPaths :many +SELECT id, library_id, file_path, group_key FROM audio_files +WHERE file_path IN (/*SLICE:paths*/?) +` + +type GetAudioFilesByPathsRow struct { + ID int64 + LibraryID int64 + FilePath string + GroupKey string +} + +func (q *Queries) GetAudioFilesByPaths(ctx context.Context, paths []string) ([]GetAudioFilesByPathsRow, error) { + query := getAudioFilesByPaths + var queryParams []interface{} + if len(paths) > 0 { + for _, v := range paths { + queryParams = append(queryParams, v) + } + query = strings.Replace(query, "/*SLICE:paths*/?", strings.Repeat(",?", len(paths))[1:], 1) + } else { + query = strings.Replace(query, "/*SLICE:paths*/?", "NULL", 1) + } + rows, err := q.db.QueryContext(ctx, query, queryParams...) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetAudioFilesByPathsRow + for rows.Next() { + var i GetAudioFilesByPathsRow + if err := rows.Scan( + &i.ID, + &i.LibraryID, + &i.FilePath, + &i.GroupKey, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getAudioFilesByReleaseGroup = `-- name: GetAudioFilesByReleaseGroup :many SELECT af.file_path, diff --git a/backend/database/sql/sqlcgen/excluded_paths.sql.go b/backend/database/sql/sqlcgen/excluded_paths.sql.go new file mode 100644 index 0000000..c4babef --- /dev/null +++ b/backend/database/sql/sqlcgen/excluded_paths.sql.go @@ -0,0 +1,75 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.30.0 +// source: excluded_paths.sql + +package sqlcgen + +import ( + "context" +) + +const clearExcludedPaths = `-- name: ClearExcludedPaths :exec +DELETE FROM excluded_paths +` + +func (q *Queries) ClearExcludedPaths(ctx context.Context) error { + _, err := q.db.ExecContext(ctx, clearExcludedPaths) + return err +} + +const countExcludedPathsByLibrary = `-- name: CountExcludedPathsByLibrary :one +SELECT COUNT(*) FROM excluded_paths +WHERE library_id = ? +` + +func (q *Queries) CountExcludedPathsByLibrary(ctx context.Context, libraryID int64) (int64, error) { + row := q.db.QueryRowContext(ctx, countExcludedPathsByLibrary, libraryID) + var count int64 + err := row.Scan(&count) + return count, err +} + +const excludePath = `-- name: ExcludePath :exec +INSERT INTO excluded_paths (library_id, file_path) +VALUES (?, ?) +ON CONFLICT(library_id, file_path) DO NOTHING +` + +type ExcludePathParams struct { + LibraryID int64 + FilePath string +} + +func (q *Queries) ExcludePath(ctx context.Context, arg ExcludePathParams) error { + _, err := q.db.ExecContext(ctx, excludePath, arg.LibraryID, arg.FilePath) + return err +} + +const getExcludedPathsByLibrary = `-- name: GetExcludedPathsByLibrary :many +SELECT file_path FROM excluded_paths +WHERE library_id = ? +` + +func (q *Queries) GetExcludedPathsByLibrary(ctx context.Context, libraryID int64) ([]string, error) { + rows, err := q.db.QueryContext(ctx, getExcludedPathsByLibrary, libraryID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var file_path string + if err := rows.Scan(&file_path); err != nil { + return nil, err + } + items = append(items, file_path) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/backend/database/sql/sqlcgen/models.go b/backend/database/sql/sqlcgen/models.go index 46cfb90..774232d 100644 --- a/backend/database/sql/sqlcgen/models.go +++ b/backend/database/sql/sqlcgen/models.go @@ -139,6 +139,13 @@ type DownloadRequest struct { UpdatedAt time.Time } +type ExcludedPath struct { + ID int64 + LibraryID int64 + FilePath string + ExcludedAt time.Time +} + type ExploreChampionFt struct { Title string ArtistName string diff --git a/backend/datamap/datamap.go b/backend/datamap/datamap.go index 8fa87ef..fd2f20f 100644 --- a/backend/datamap/datamap.go +++ b/backend/datamap/datamap.go @@ -193,6 +193,14 @@ var tables = []Table{ Note: "Build metadata for explore_index: dump version, coverage " + "tiers, last refresh.", }, + { + Name: "excluded_paths", Kind: Authored, Lifetime: Cascade, + Note: "Paths the user removed from the library, which the scanner " + + "must not import again. Authored: it is a decision, not " + + "derivable from disk. Cascades with its library, and a full " + + "rescan clears it \u2014 the only way back for a path removed by " + + "mistake.", + }, { Name: "file_types", Kind: Derived, Lifetime: Retained, Note: "Static lookup rows seeded from code, not user data.", diff --git a/backend/datamap/datamap_test.go b/backend/datamap/datamap_test.go index d89f3f4..ee0425f 100644 --- a/backend/datamap/datamap_test.go +++ b/backend/datamap/datamap_test.go @@ -240,6 +240,14 @@ func TestAuthoredCascadesAreDeliberate(t *testing.T) { // unsubscribing from an artist must stop the albums it queued // on the user's behalf. "download_requests": true, + + // An exclusion says "do not import this path into library 3". + // Remove that library and no scan will ever visit the path + // again, so the row has nothing left to exclude it from — and + // the data it protects is the *absence* of a row, which the + // library removal has already achieved for everything. Adding + // the library back is the user asking to import it afresh. + "excluded_paths": true, } for _, entry := range datamap.ByKind(datamap.Authored) { diff --git a/backend/events/events.go b/backend/events/events.go index 6e13d37..f7d6365 100644 --- a/backend/events/events.go +++ b/backend/events/events.go @@ -100,6 +100,19 @@ const ( BatchWriteProgress = "BatchWriteProgress" ) +// Track removal events. +// +// TracksRemovedFromLibrary means "these rows are gone and these paths +// will not be imported again", and like TrackPlayCountChanged it +// carries everything a consumer needs to patch rather than invalidate: +// {filePaths: []string, count: int}. The library store splices those +// paths out of its tracks array — which is the expensive collection — +// and refetches only the album/artist/genre summaries, whose counts +// really did change. +const ( + TracksRemovedFromLibrary = "TracksRemovedFromLibrary" +) + // Play statistics events. // // TrackPlayCountChanged carries everything needed to patch the one diff --git a/backend/library/exclusions.go b/backend/library/exclusions.go new file mode 100644 index 0000000..bf5d116 --- /dev/null +++ b/backend/library/exclusions.go @@ -0,0 +1,92 @@ +package library + +import ( + "fmt" + "path/filepath" + "strings" + + "yellowjacket/backend/database/sql/sqlcgen" +) + +// libraryRoot is one configured library's id and root directory. +type libraryRoot struct { + id int64 + path string +} + +// libraryRoots returns every configured library's id and root path. +func (l *Library) libraryRoots() ([]libraryRoot, error) { + libs, err := l.db.Queries.GetAllLibraries(l.ctx) + if err != nil { + return nil, fmt.Errorf("could not load libraries: %w", err) + } + + roots := make([]libraryRoot, 0, len(libs)) + for _, lib := range libs { + roots = append(roots, libraryRoot{id: lib.ID, path: lib.Path}) + } + + return roots, nil +} + +// pathWithin reports whether path lies under root. Both are cleaned +// first, and the comparison keeps the separator so /music/rock does not +// swallow /music/rockabilly. +func pathWithin(root, path string) bool { + cleanRoot := filepath.Clean(root) + cleanPath := filepath.Clean(path) + + if cleanRoot == cleanPath { + return true + } + + return strings.HasPrefix(cleanPath, cleanRoot+string(filepath.Separator)) +} + +// excludeParams is the insert parameter for one exclusion. +func excludeParams(libraryID int64, filePath string) sqlcgen.ExcludePathParams { + return sqlcgen.ExcludePathParams{ + LibraryID: libraryID, + FilePath: filePath, + } +} + +// excludedPathSet loads a library's excluded paths as a set, cleaned +// the same way the walk builds its absolute paths so the two compare. +// +// A failure here returns an empty set and logs: a scan that cannot read +// the exclusions imports what it finds, which is the pre-exclusion +// behaviour rather than an empty library. +func (l *Library) excludedPathSet(libraryID int64) map[string]struct{} { + paths, err := l.db.Queries.GetExcludedPathsByLibrary(l.ctx, libraryID) + if err != nil { + l.logger.Warn("could not load excluded paths, scanning everything", + "libraryID", libraryID, "err", err) + + return nil + } + + if len(paths) == 0 { + return nil + } + + set := make(map[string]struct{}, len(paths)) + for _, p := range paths { + set[filepath.Clean(p)] = struct{}{} + } + + return set +} + +// isExcluded reports whether an absolute file path is in the set. A +// nil set excludes nothing, which is what every caller wants when +// there are no exclusions at all. +func isExcluded(set map[string]struct{}, absolutePath string) bool { + if len(set) == 0 { + return false + } + + _, found := set[filepath.Clean(absolutePath)] + + return found +} diff --git a/backend/library/library.go b/backend/library/library.go index 72333ea..2a7a367 100644 --- a/backend/library/library.go +++ b/backend/library/library.go @@ -334,7 +334,13 @@ func (l *Library) scanInternal( // --- Pre-walk: count audio files for progress reporting --- emitProgress(mkProgress("counting", 0, 0, 0, 0, 0)) - totalFiles := countAudioFiles(basePath) + // Paths the user has removed from the library. Loaded once per + // scan: the walk consults it per file, and the counts above and + // below must agree with it or the progress bar and the soft scan + // both describe a library that is not the one being built. + excluded := l.excludedPathSet(libraryID) + + totalFiles := countAudioFiles(basePath, excluded) l.logger.Debug( "pre-walk file count complete", @@ -423,6 +429,20 @@ func (l *Library) scanInternal( return nil } + // The user removed this path from the library. Leaving + // it out of existingPaths' LoadAndDelete as well is + // deliberate: if a row somehow exists for an excluded + // path, orphan cleanup below deletes it, which is the + // state the user asked for. + if isExcluded(excluded, absoluteFilePath) { + l.logger.Debug( + "skipping excluded path", + "path", absoluteFilePath, + ) + + return nil + } + // Stat the entry for the staleness comparison below. // This happens before the file is read, so a file // modified mid-scan records the pre-read mtime and is @@ -1088,21 +1108,27 @@ func (l *Library) pruneOrphanedMetadata() { // countAudioFiles performs a fast walk of the library directory, // counting only files with supported audio extensions. No per-file // I/O is performed — this reads only directory entries. -func countAudioFiles(basePath string) int64 { +func countAudioFiles(basePath string, excluded map[string]struct{}) int64 { var count int64 _ = fs.WalkDir( os.DirFS(basePath), ".", - func(_ string, d fs.DirEntry, err error) error { + func(path string, d fs.DirEntry, err error) error { if err != nil || d.IsDir() { return nil } ext := filepath.Ext(d.Name()) - if _, ok := metadata.GetSupportedFileType(ext); ok { - count++ + if _, ok := metadata.GetSupportedFileType(ext); !ok { + return nil } + if isExcluded(excluded, filepath.Join(basePath, path)) { + return nil + } + + count++ + return nil }, ) @@ -1119,10 +1145,17 @@ func countAudioFiles(basePath string) int64 { // Unlike countAudioFiles this stats every entry, so it is the more // expensive of the two walks. Only the startup soft scan uses it — // the in-scan progress total does not need mtimes. -func surveyAudioFiles(basePath string) (count, maxModTime int64) { +// +// Both walks take the library's excluded paths and skip them, because +// both answer "how many files would a scan import", not "how many +// files are there". +func surveyAudioFiles( + basePath string, + excluded map[string]struct{}, +) (count, maxModTime int64) { _ = fs.WalkDir( os.DirFS(basePath), ".", - func(_ string, d fs.DirEntry, err error) error { + func(path string, d fs.DirEntry, err error) error { if err != nil || d.IsDir() { return nil } @@ -1132,6 +1165,14 @@ func surveyAudioFiles(basePath string) (count, maxModTime int64) { return nil } + // An excluded path is not a file this scan would import, + // so it must not be counted: the soft scan compares this + // count against the database's, and a permanent + // disagreement queues a full scan on every launch. + if isExcluded(excluded, filepath.Join(basePath, path)) { + return nil + } + count++ info, infoErr := d.Info() diff --git a/backend/library/remove_tracks.go b/backend/library/remove_tracks.go new file mode 100644 index 0000000..7e40ca1 --- /dev/null +++ b/backend/library/remove_tracks.go @@ -0,0 +1,190 @@ +package library + +import ( + "errors" + "fmt" + + "yellowjacket/backend/events" +) + +// errNoPathsToRemove is returned when RemoveFromLibrary is called with +// nothing to remove. A static error because err113 forbids a dynamic +// one, and a sentinel because the frontend distinguishes it. +var errNoPathsToRemove = errors.New("no file paths given to remove") + +// RemovalResult reports what one RemoveFromLibrary call did. +type RemovalResult struct { + // TracksRemoved is how many audio_files rows were deleted. It can + // be lower than len(filePaths) if a path was already gone. + TracksRemoved int64 `json:"tracksRemoved"` + // PathsExcluded is how many paths the scanner will now skip. + PathsExcluded int64 `json:"pathsExcluded"` +} + +// RemoveFromLibrary deletes the database rows for the given file paths +// and records each path as excluded, so the next scan does not import +// it again. **It does not touch the files on disk** — that is the +// promise the confirmation dialog makes, and the reason this operation +// is safe to put one keystroke from a focused row. +// +// The exclusion is not an enhancement. Without it the next scan finds +// the file, sees no row, and imports it again — a button that undoes +// itself, which is worse than no button. +func (l *Library) RemoveFromLibrary(filePaths []string) (*RemovalResult, error) { + if len(filePaths) == 0 { + return nil, errNoPathsToRemove + } + + rows, err := l.db.Queries.GetAudioFilesByPaths(l.ctx, filePaths) + if err != nil { + return nil, fmt.Errorf("could not resolve paths to remove: %w", err) + } + + tx, err := l.db.BeginTx() + if err != nil { + return nil, fmt.Errorf("could not begin removal transaction: %w", err) + } + + committed := false + + defer func() { + if !committed { + _ = tx.Rollback() + } + }() + + txq := l.db.Queries.WithTx(tx) + + var result RemovalResult + + // Exclude every path the caller named, including one whose row has + // already gone: the user asked for that file to stay out, and a row + // that disappeared between the click and the commit is not a reason + // to let the next scan bring it back. A path with no row at all is + // attributed to the library that contains it, resolved below. + rowByPath := make(map[string]int64, len(rows)) + + for _, row := range rows { + rowByPath[row.FilePath] = row.LibraryID + + if err := txq.DeleteAudioFile(l.ctx, row.ID); err != nil { + return nil, fmt.Errorf("could not delete audio file row: %w", err) + } + + result.TracksRemoved++ + + // Keep the file's tagging group in sync, exactly as the scan's + // orphan cleanup does: drop the count and clear the group once + // it is empty, or the autotag queue keeps a row counting files + // that no longer exist. + if row.GroupKey != "" { + if err := txq.DecrementTaggingItemTrackCount(l.ctx, row.GroupKey); err != nil { + return nil, fmt.Errorf("could not decrement tagging group: %w", err) + } + + if err := txq.DeleteTaggingItemIfEmpty(l.ctx, row.GroupKey); err != nil { + return nil, fmt.Errorf("could not clear emptied tagging group: %w", err) + } + } + } + + libraryIDs, err := l.libraryIDsForPaths(filePaths, rowByPath) + if err != nil { + return nil, err + } + + for _, path := range filePaths { + libraryID, known := libraryIDs[path] + if !known { + // Outside every configured library: no scan will ever + // visit it, so there is nothing to exclude it from. + l.logger.Debug("removal: path belongs to no library, not excluding", + "path", path) + + continue + } + + if err := txq.ExcludePath(l.ctx, excludeParams(libraryID, path)); err != nil { + return nil, fmt.Errorf("could not exclude path: %w", err) + } + + result.PathsExcluded++ + } + + if err := tx.Commit(); err != nil { + return nil, fmt.Errorf("could not commit removal: %w", err) + } + + committed = true + + // Post-commit, and best-effort: the rows are gone either way, and a + // failure here leaves stale index entries rather than a wrong + // library. The FTS5 index is contentless, so a stale entry is + // harmless — searches join track_metadata, which no longer has the + // row — but removing it keeps the index from growing forever. + for _, row := range rows { + if err := l.db.DeleteSearchIndex(row.ID); err != nil { + l.logger.Warn("could not delete FTS entry for removed track", + "path", row.FilePath, "id", row.ID, "err", err) + } + } + + // An album, artist or genre whose last track just went is now a row + // with nothing behind it, and the album list selects from + // release_groups rather than from audio_files — so it would keep + // rendering an album the user has no tracks of. + l.pruneOrphanedMetadata() + + l.emit(events.TracksRemovedFromLibrary, map[string]any{ + "filePaths": filePaths, + "count": result.TracksRemoved, + }) + + l.logger.Info("removed tracks from library", + "requested", len(filePaths), + "rowsDeleted", result.TracksRemoved, + "pathsExcluded", result.PathsExcluded, + ) + + return &result, nil +} + +// libraryIDsForPaths maps each path to the library it belongs to. A +// path that still had a row takes that row's library_id; one that did +// not is matched against the configured library roots by prefix, which +// is what the scan walk would do with it. +func (l *Library) libraryIDsForPaths( + filePaths []string, + rowByPath map[string]int64, +) (map[string]int64, error) { + out := make(map[string]int64, len(filePaths)) + + var libs []libraryRoot + + for _, path := range filePaths { + if libraryID, ok := rowByPath[path]; ok { + out[path] = libraryID + + continue + } + + if libs == nil { + var err error + + libs, err = l.libraryRoots() + if err != nil { + return nil, err + } + } + + for _, lib := range libs { + if pathWithin(lib.path, path) { + out[path] = lib.id + + break + } + } + } + + return out, nil +} diff --git a/backend/library/remove_tracks_test.go b/backend/library/remove_tracks_test.go new file mode 100644 index 0000000..cd764a9 --- /dev/null +++ b/backend/library/remove_tracks_test.go @@ -0,0 +1,270 @@ +package library + +import ( + "log/slog" + "os" + "path/filepath" + "slices" + "testing" + + "yellowjacket/backend/database" + "yellowjacket/backend/database/sql/sqlcgen" + "yellowjacket/backend/events" + "yellowjacket/internal/testfixtures" +) + +// setupScanLibrary builds a Library over a temp directory holding +// copies of `count` real fixture tracks, plus the libraries row the +// scan needs. Real files, because the scan extracts tags from them. +func setupScanLibrary( + t *testing.T, + count int, +) (lib *Library, dir string, paths []string, rec *events.Recorder, libID int64) { + t.Helper() + + m := testfixtures.Load(t) + sources := m.Case(t, testfixtures.CaseFLACAlbum) + + if len(sources) < count { + t.Fatalf("fixture case has %d tracks, need %d", len(sources), count) + } + + dir = t.TempDir() + paths = make([]string, 0, count) + + for _, src := range sources[:count] { + data, err := os.ReadFile(src) + if err != nil { + t.Fatalf("read fixture %s: %v", src, err) + } + + dst := filepath.Join(dir, filepath.Base(src)) + if err := os.WriteFile(dst, data, 0o600); err != nil { + t.Fatalf("write fixture copy: %v", err) + } + + paths = append(paths, dst) + } + + db := database.NewTestDB(t) + rec = events.NewRecorder() + + lib = &Library{ + ctx: events.WithSink(t.Context(), rec), + logger: slog.Default(), + conf: &Config{}, + db: db, + } + + row, err := db.Queries.CreateLibrary(t.Context(), sqlcgen.CreateLibraryParams{ + Name: "Test", + Path: dir, + }) + if err != nil { + t.Fatalf("create library row: %v", err) + } + + slices.Sort(paths) + + return lib, dir, paths, rec, row.ID +} + +// scannedPaths returns the file paths currently in the database, sorted. +func scannedPaths(t *testing.T, lib *Library) []string { + t.Helper() + + rows, err := lib.db.Queries.GetAllAudioFilePaths(t.Context()) + if err != nil { + t.Fatalf("read audio file paths: %v", err) + } + + out := make([]string, 0, len(rows)) + for _, row := range rows { + out = append(out, row.FilePath) + } + + slices.Sort(out) + + return out +} + +// TestRemoveFromLibrary_SurvivesRescan is the assertion the whole phase +// rests on: a removed path stays removed across a real scan of the real +// directory, and the file it named is still on disk. +// +// Its positive half is not optional. A guard that excluded everything +// would satisfy "the removed path did not come back" for free, so the +// same scan must also put back a row deleted *without* an exclusion. +func TestRemoveFromLibrary_SurvivesRescan(t *testing.T) { + t.Parallel() + + lib, dir, paths, _, libID := setupScanLibrary(t, 3) + + lib.scanInternal(libID, "Test", dir) + + if got := scannedPaths(t, lib); len(got) != 3 { + t.Fatalf("first scan imported %d tracks, want 3: %v", len(got), got) + } + + removed := paths[0] + // The control: its row is deleted directly, with no exclusion, so + // the same scan has to bring it back. + control := paths[1] + + result, err := lib.RemoveFromLibrary([]string{removed}) + if err != nil { + t.Fatalf("RemoveFromLibrary: %v", err) + } + + if result.TracksRemoved != 1 || result.PathsExcluded != 1 { + t.Fatalf( + "RemoveFromLibrary = %+v, want 1 removed and 1 excluded", + result, + ) + } + + controlRow, err := lib.db.Queries.GetAudioFileByPath(t.Context(), control) + if err != nil { + t.Fatalf("look up control track: %v", err) + } + + if err := lib.db.Queries.DeleteAudioFile(t.Context(), controlRow.ID); err != nil { + t.Fatalf("delete control row: %v", err) + } + + lib.scanInternal(libID, "Test", dir) + + after := scannedPaths(t, lib) + + if slices.Contains(after, removed) { + t.Errorf("excluded path came back after a rescan: %s\nrows: %v", removed, after) + } + + if !slices.Contains(after, control) { + t.Errorf( + "the rescan did not re-import a path that was NOT excluded (%s)"+ + " — the exclusion is skipping more than it was asked to\nrows: %v", + control, after, + ) + } + + // The promise the confirmation dialog makes. + if _, err := os.Stat(removed); err != nil { + t.Errorf("removed file is no longer on disk: %v", err) + } +} + +// TestRemoveFromLibrary_SoftScanSeesNoChange pins the trap that would +// otherwise queue a full scan on every launch: the soft scan compares +// the number of audio files on disk against the number of rows, and an +// excluded path is on disk and deliberately not a row. +func TestRemoveFromLibrary_SoftScanSeesNoChange(t *testing.T) { + t.Parallel() + + lib, dir, paths, _, libID := setupScanLibrary(t, 3) + + lib.scanInternal(libID, "Test", dir) + + if _, err := lib.RemoveFromLibrary([]string{paths[0]}); err != nil { + t.Fatalf("RemoveFromLibrary: %v", err) + } + + dbCount, err := lib.db.Queries.CountAudioFilesByLibrary(t.Context(), libID) + if err != nil { + t.Fatalf("count rows: %v", err) + } + + diskCount, _ := surveyAudioFiles(dir, lib.excludedPathSet(libID)) + + if diskCount != dbCount { + t.Errorf( + "soft scan would see disk %d vs db %d — every launch queues a full scan", + diskCount, dbCount, + ) + } + + // And the positive half: without the exclusion set the survey still + // counts the file, which is what makes the argument above real + // rather than a tautology about a function that counts nothing. + if raw, _ := surveyAudioFiles(dir, nil); raw != dbCount+1 { + t.Errorf("unfiltered survey = %d, want %d", raw, dbCount+1) + } +} + +// TestRemoveFromLibrary_FullRescanClearsExclusions pins the only route +// back for a path removed by mistake. +func TestRemoveFromLibrary_FullRescanClearsExclusions(t *testing.T) { + t.Parallel() + + lib, dir, paths, _, libID := setupScanLibrary(t, 2) + + lib.scanInternal(libID, "Test", dir) + + if _, err := lib.RemoveFromLibrary([]string{paths[0]}); err != nil { + t.Fatalf("RemoveFromLibrary: %v", err) + } + + if err := lib.clearLibraryTables(); err != nil { + t.Fatalf("clearLibraryTables: %v", err) + } + + count, err := lib.db.Queries.CountExcludedPathsByLibrary(t.Context(), libID) + if err != nil { + t.Fatalf("count exclusions: %v", err) + } + + if count != 0 { + t.Fatalf("full rescan left %d exclusions behind", count) + } + + lib.scanInternal(libID, "Test", dir) + + if !slices.Contains(scannedPaths(t, lib), paths[0]) { + t.Error("a full rescan did not bring back a previously excluded path") + } +} + +// TestRemoveFromLibrary_EmitsPatchablePayload checks the event carries +// what a store needs to patch rather than invalidate. +func TestRemoveFromLibrary_EmitsPatchablePayload(t *testing.T) { + t.Parallel() + + lib, dir, paths, rec, libID := setupScanLibrary(t, 2) + + lib.scanInternal(libID, "Test", dir) + + if _, err := lib.RemoveFromLibrary([]string{paths[0]}); err != nil { + t.Fatalf("RemoveFromLibrary: %v", err) + } + + ev, ok := rec.Last(events.TracksRemovedFromLibrary) + if !ok { + t.Fatalf("no TracksRemovedFromLibrary emitted; got %v", rec.Names()) + } + + payload, ok := ev.Payload().(map[string]any) + if !ok { + t.Fatalf("payload is %T, want a map", ev.Payload()) + } + + got, ok := payload["filePaths"].([]string) + if !ok || len(got) != 1 || got[0] != paths[0] { + t.Errorf("payload filePaths = %v, want [%s]", payload["filePaths"], paths[0]) + } + + if count, ok := payload["count"].(int64); !ok || count != 1 { + t.Errorf("payload count = %v, want 1", payload["count"]) + } +} + +// TestRemoveFromLibrary_RejectsAnEmptyRequest keeps a stray Delete on +// an empty selection from reaching the database at all. +func TestRemoveFromLibrary_RejectsAnEmptyRequest(t *testing.T) { + t.Parallel() + + lib, _, _, _, _ := setupScanLibrary(t, 1) + + if _, err := lib.RemoveFromLibrary(nil); err == nil { + t.Error("RemoveFromLibrary(nil) succeeded, want an error") + } +} diff --git a/backend/library/rescan.go b/backend/library/rescan.go index b8e75bb..9b6ef63 100644 --- a/backend/library/rescan.go +++ b/backend/library/rescan.go @@ -124,6 +124,15 @@ func (l *Library) clearLibraryTables() error { return fmt.Errorf("could not clear queue tracks: %w", err) } + // A full rescan is the "start over" button, and it is the only way + // back for a path removed from the library by mistake: the file is + // still on disk, but nothing else will ever import it again while + // its exclusion stands. Until there is a UI for managing the list, + // clearing it here is the escape hatch. + if err := txq.ClearExcludedPaths(l.ctx); err != nil { + return fmt.Errorf("could not clear excluded paths: %w", err) + } + // Preserve playlist tracks across rescan: populate phantom // metadata for all linked tracks before audio_files are deleted. // ON DELETE SET NULL will null out audio_file_id, converting them diff --git a/backend/library/scan_queue.go b/backend/library/scan_queue.go index 27ccdef..737958b 100644 --- a/backend/library/scan_queue.go +++ b/backend/library/scan_queue.go @@ -181,7 +181,12 @@ func (l *Library) SoftScanAllLibraries() error { continue } - diskCount, diskModTime := surveyAudioFiles(lib.Path) + // Excluded paths are on disk and deliberately not in the + // database, so the survey must skip them or the two counts + // disagree forever and every launch queues a full scan. + diskCount, diskModTime := surveyAudioFiles( + lib.Path, l.excludedPathSet(lib.ID), + ) // A newer file on disk than anything on record means something // was edited in place since the last scan. An older newest-mtime diff --git a/backend/library/staleness_test.go b/backend/library/staleness_test.go index 621dd77..f98dcd3 100644 --- a/backend/library/staleness_test.go +++ b/backend/library/staleness_test.go @@ -130,7 +130,7 @@ func TestSurveyAudioFiles(t *testing.T) { // the result, or every artwork change would trigger a rescan. setModTime(t, filepath.Join(dir, "cover.jpg"), time.Now()) - count, maxMod := surveyAudioFiles(dir) + count, maxMod := surveyAudioFiles(dir, nil) if count != 2 { t.Errorf("count = %d, want 2", count) @@ -145,7 +145,7 @@ func TestSurveyAudioFiles(t *testing.T) { touched := time.Now() setModTime(t, filepath.Join(dir, "a.mp3"), touched) - _, afterMod := surveyAudioFiles(dir) + _, afterMod := surveyAudioFiles(dir, nil) if afterMod != touched.Unix() { t.Errorf( @@ -158,7 +158,7 @@ func TestSurveyAudioFiles(t *testing.T) { func TestSurveyAudioFiles_EmptyDir(t *testing.T) { t.Parallel() - count, maxMod := surveyAudioFiles(t.TempDir()) + count, maxMod := surveyAudioFiles(t.TempDir(), nil) if count != 0 || maxMod != 0 { t.Errorf( diff --git a/frontend/src/events.ts b/frontend/src/events.ts index 5a71e7c..eeb2157 100644 --- a/frontend/src/events.ts +++ b/frontend/src/events.ts @@ -66,6 +66,17 @@ export const Events = { TrackMetadataChanged: "TrackMetadataChanged", BatchWriteProgress: "BatchWriteProgress", + // Track removal events. + // + // TracksRemovedFromLibrary means "these rows are gone and these paths + // will not be imported again", and like TrackPlayCountChanged it + // carries everything a consumer needs to patch rather than invalidate: + // {filePaths: []string, count: int}. The library store splices those + // paths out of its tracks array — which is the expensive collection — + // and refetches only the album/artist/genre summaries, whose counts + // really did change + TracksRemovedFromLibrary: "TracksRemovedFromLibrary", + // Play statistics events. // // TrackPlayCountChanged carries everything needed to patch the one diff --git a/frontend/wailsjs/go/library/Library.d.ts b/frontend/wailsjs/go/library/Library.d.ts index dd343d3..1899c61 100755 --- a/frontend/wailsjs/go/library/Library.d.ts +++ b/frontend/wailsjs/go/library/Library.d.ts @@ -71,6 +71,8 @@ export function QueuedLibraryNames():Promise>; export function ReleasePipelineLock():Promise; +export function RemoveFromLibrary(arg1:Array):Promise; + export function RemoveLibrary(arg1:number):Promise; export function RenameLibrary(arg1:number,arg2:string):Promise; diff --git a/frontend/wailsjs/go/library/Library.js b/frontend/wailsjs/go/library/Library.js index bbfa8f2..2f355e5 100755 --- a/frontend/wailsjs/go/library/Library.js +++ b/frontend/wailsjs/go/library/Library.js @@ -134,6 +134,10 @@ export function ReleasePipelineLock() { return window['go']['library']['Library']['ReleasePipelineLock'](); } +export function RemoveFromLibrary(arg1) { + return window['go']['library']['Library']['RemoveFromLibrary'](arg1); +} + export function RemoveLibrary(arg1) { return window['go']['library']['Library']['RemoveLibrary'](arg1); } diff --git a/frontend/wailsjs/go/models.ts b/frontend/wailsjs/go/models.ts index 949c39a..f9903c4 100755 --- a/frontend/wailsjs/go/models.ts +++ b/frontend/wailsjs/go/models.ts @@ -1760,6 +1760,20 @@ export namespace library { this.queueItemCount = source["queueItemCount"]; } } + export class RemovalResult { + tracksRemoved: number; + pathsExcluded: number; + + static createFrom(source: any = {}) { + return new RemovalResult(source); + } + + constructor(source: any = {}) { + if ('string' === typeof source) source = JSON.parse(source); + this.tracksRemoved = source["tracksRemoved"]; + this.pathsExcluded = source["pathsExcluded"]; + } + } export class RemovalSummary { tracksDeleted: number; artistsRemoved: number;