diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index aa57a6a7..d447a7bf 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -122,7 +122,15 @@ func run() error { } defer pool.Close() - scanner := library.New(pool, logger, cfg.Library.ScanPaths) + // Fingerprinting settings (M400 #3913): one instance, shared by the scanner, + // the fingerprint backfill, the duplicate sweep and the admin API, so a save + // reaches all of them without a restart. A load failure is logged, not fatal: + // the service falls back to the shipped defaults. + fpSettings, fpErr := library.NewFingerprintSettingsService(ctx, pool) + if fpErr != nil { + logger.Warn("fingerprint settings: using defaults", "err", fpErr) + } + scanner := library.New(pool, logger, cfg.Library.ScanPaths, fpSettings) contact := cfg.Library.ContactEmail if contact == "" { @@ -218,7 +226,12 @@ func run() error { // will — everything imported before fingerprinting existed, and rows derived // by an older method. A worker of its own rather than a scan stage; see // internal/library/fingerprint_backfill.go for why. - go library.NewFingerprintBackfillWorker(pool, logger.With("component", "fingerprint_backfill")).Run(ctx) + go library.NewFingerprintBackfillWorker(pool, logger.With("component", "fingerprint_backfill"), fpSettings).Run(ctx) + + // Duplicate sweep (M400 #3910): proposes groups of tracks holding one + // recording, from the fingerprints above. Sweeps only when fingerprints have + // changed since the last sweep. + go library.NewDuplicateSweepWorker(pool, logger.With("component", "duplicate_sweep"), fpSettings).Run(ctx) // Start the tag-enrichment worker (#1490). Reconciles the compiled-in // tag providers with tag_provider_settings, bumps the sources version if @@ -363,6 +376,10 @@ func run() error { srv.PlaylistScheduler = playlistScheduler srv.RecSettings = recSettings srv.TagSettings = tagSettings + srv.FingerprintSettings = fpSettings + // The sweeper above holds this same instance, so a save from the admin + // card changes what it does on its next tick (#3936). + srv.ReacqSettings = reacqSettings srv.StreamSecret = cfg.StreamSecret httpServer := &http.Server{ Addr: cfg.Server.Address, diff --git a/internal/api/admin_coverage.go b/internal/api/admin_coverage.go index 3c5e0d58..7bbe259c 100644 --- a/internal/api/admin_coverage.go +++ b/internal/api/admin_coverage.go @@ -1,6 +1,7 @@ package api import ( + "context" "net/http" "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" @@ -40,12 +41,32 @@ func (h *handlers) handleGetLibraryCoverage(w http.ResponseWriter, r *http.Reque // fingerprintCoverageResp is the wire shape for GET /api/admin/library/fingerprints. // fingerprinted + rejected + pending = total. Missing tracks are not counted: -// there is no file to fingerprint. +// there is no file to fingerprint. Enabled travels with the counts because with +// fingerprinting off (#3913) pending never shrinks, and a gauge that implies +// progress would be promising work nothing is doing. type fingerprintCoverageResp struct { Total int64 `json:"total"` Fingerprinted int64 `json:"fingerprinted"` Rejected int64 `json:"rejected"` Pending int64 `json:"pending"` + Enabled bool `json:"enabled"` +} + +// fingerprintCoverage reads the gauge against the current settings: a print at +// another length counts as pending, because the backfill will re-derive it. +func (h *handlers) fingerprintCoverage(ctx context.Context) (fingerprintCoverageResp, error) { + cfg := h.fingerprintSettings.Get() + row, err := library.FingerprintCoverage(ctx, h.pool, cfg) + if err != nil { + return fingerprintCoverageResp{}, err + } + return fingerprintCoverageResp{ + Total: row.Total, + Fingerprinted: row.Fingerprinted, + Rejected: row.Rejected, + Pending: row.Pending, + Enabled: cfg.Enabled, + }, nil } // handleGetFingerprintCoverage implements GET /api/admin/library/fingerprints: @@ -53,15 +74,10 @@ type fingerprintCoverageResp struct { // worker spanning many passes, with no scan run to attach a tally to, so its // progress is read live here. Always 200; zeros on an empty library. func (h *handlers) handleGetFingerprintCoverage(w http.ResponseWriter, r *http.Request) { - row, err := library.FingerprintCoverage(r.Context(), h.pool) + cov, err := h.fingerprintCoverage(r.Context()) if err != nil { writeErrWithLog(w, h.logger, "admin: get fingerprint coverage", apierror.InternalMsg("lookup failed", err)) return } - writeJSON(w, http.StatusOK, fingerprintCoverageResp{ - Total: row.Total, - Fingerprinted: row.Fingerprinted, - Rejected: row.Rejected, - Pending: row.Pending, - }) + writeJSON(w, http.StatusOK, cov) } diff --git a/internal/api/admin_duplicates.go b/internal/api/admin_duplicates.go new file mode 100644 index 00000000..cbe2809d --- /dev/null +++ b/internal/api/admin_duplicates.go @@ -0,0 +1,316 @@ +package api + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + + "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" +) + +// duplicateMemberView is one copy in a proposed duplicate group. LikeCount and +// PlayCount span every user: the report is admin-only, and what a copy carries +// is the fact the operator weighs when choosing which to keep. +type duplicateMemberView struct { + TrackID string `json:"track_id"` + Title string `json:"title"` + ArtistName string `json:"artist_name"` + AlbumID string `json:"album_id"` + AlbumTitle string `json:"album_title"` + FilePath string `json:"file_path"` + FileFormat string `json:"file_format"` + FileSize int64 `json:"file_size"` + DurationSec int32 `json:"duration_sec"` + AddedAt string `json:"added_at"` + LikeCount int64 `json:"like_count"` + PlayCount int64 `json:"play_count"` +} + +// duplicateGroupView is one proposal. SurvivorTrackID and SurvivorReason are +// the copy the report proposes keeping and the rule that chose it +// (library.ProposeSurvivor) — a default the merge (#3911) lets the operator +// override. +type duplicateGroupView struct { + ID string `json:"id"` + Tier string `json:"tier"` + WorstBitErrorRate *float32 `json:"worst_bit_error_rate"` + DetectedAt string `json:"detected_at"` + SurvivorTrackID string `json:"survivor_track_id"` + SurvivorReason string `json:"survivor_reason"` + Members []duplicateMemberView `json:"members"` +} + +// duplicateSweepView is the latest sweep. State is "never" when none has run, +// which is what lets the page tell an empty report apart from a sweep that +// found nothing. +type duplicateSweepView struct { + State string `json:"state"` + StartedAt *string `json:"started_at"` + FinishedAt *string `json:"finished_at"` + Candidates *int32 `json:"candidates"` + GroupsFound *int32 `json:"groups_found"` + OversizeClusters *int32 `json:"oversize_clusters"` + ErrorMessage *string `json:"error_message"` +} + +// adminDuplicatesResponse is the paged report. Total counts groups. +type adminDuplicatesResponse struct { + Sweep duplicateSweepView `json:"sweep"` + Fingerprints fingerprintCoverageResp `json:"fingerprints"` + Total int64 `json:"total"` + Limit int `json:"limit"` + Offset int `json:"offset"` + Groups []duplicateGroupView `json:"groups"` +} + +// handleListDuplicates implements GET /api/admin/library/duplicates (#3912). +// +// Read-only. The sweep's state and the fingerprint backfill's progress travel +// with the groups because an empty report means three different things — still +// fingerprinting, never swept, or swept and clean — and the page has to say which. +func (h *handlers) handleListDuplicates(w http.ResponseWriter, r *http.Request) { + limit, offset, err := parsePaging(r.URL.Query()) + if err != nil { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_paging") + return + } + ctx := r.Context() + q := dbq.New(h.pool) + + sweep := duplicateSweepView{State: "never"} + last, err := q.GetLatestDuplicateSweep(ctx) + switch { + case err == nil: + sweep = duplicateSweepViewOf(last) + case !errors.Is(err, pgx.ErrNoRows): + h.logger.Error("admin: latest duplicate sweep", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + + cov, err := h.fingerprintCoverage(ctx) + if err != nil { + h.logger.Error("admin: fingerprint coverage", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + total, err := q.CountPendingDuplicateGroups(ctx) + if err != nil { + h.logger.Error("admin: count duplicate groups", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + rows, err := q.ListPendingDuplicateGroupMembers(ctx, dbq.ListPendingDuplicateGroupMembersParams{ + PageLimit: int32(limit), PageOffset: int32(offset), + }) + if err != nil { + h.logger.Error("admin: list duplicate groups", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + + writeJSON(w, http.StatusOK, adminDuplicatesResponse{ + Sweep: sweep, + Fingerprints: cov, + Total: total, + Limit: limit, + Offset: offset, + Groups: foldDuplicateGroups(rows), + }) +} + +func duplicateSweepViewOf(s dbq.DuplicateSweep) duplicateSweepView { + v := duplicateSweepView{ + State: "running", + Candidates: s.Candidates, + GroupsFound: s.GroupsFound, + OversizeClusters: s.OversizeClusters, + ErrorMessage: s.ErrorMessage, + } + started := formatTimestamp(s.StartedAt) + v.StartedAt = &started + if s.FinishedAt.Valid { + finished := formatTimestamp(s.FinishedAt) + v.FinishedAt = &finished + v.State = "finished" + } + return v +} + +// foldDuplicateGroups folds the one-row-per-member query result into groups and +// proposes each group's survivor. It relies on the query ordering members of a +// group together, so a run-length fold is enough and the page order holds. +func foldDuplicateGroups(rows []dbq.ListPendingDuplicateGroupMembersRow) []duplicateGroupView { + groups := make([]duplicateGroupView, 0, 8) + var candidates [][]library.SurvivorCandidate + for _, row := range rows { + id := uuidToString(row.GroupID) + if n := len(groups); n == 0 || groups[n-1].ID != id { + groups = append(groups, duplicateGroupView{ + ID: id, + Tier: row.Tier, + WorstBitErrorRate: row.WorstBitErrorRate, + DetectedAt: formatTimestamp(row.DetectedAt), + }) + candidates = append(candidates, nil) + } + n := len(groups) - 1 + trackID := uuidToString(row.TrackID) + groups[n].Members = append(groups[n].Members, duplicateMemberView{ + TrackID: trackID, + Title: row.Title, + ArtistName: row.ArtistName, + AlbumID: uuidToString(row.AlbumID), + AlbumTitle: row.AlbumTitle, + FilePath: row.FilePath, + FileFormat: row.FileFormat, + FileSize: row.FileSize, + DurationSec: row.DurationMs / 1000, + AddedAt: formatTimestamp(row.AddedAt), + LikeCount: row.LikeCount, + PlayCount: row.PlayCount, + }) + candidates[n] = append(candidates[n], library.SurvivorCandidate{ + TrackID: trackID, FileFormat: row.FileFormat, FileSize: row.FileSize, AddedAt: row.AddedAt.Time, + }) + } + for i := range groups { + groups[i].SurvivorTrackID, groups[i].SurvivorReason = library.ProposeSurvivor(candidates[i]) + } + return groups +} + +// handleRunDuplicateSweep implements POST /api/admin/library/duplicates/sweep: +// 202 when a sweep starts, 409 sweep_in_progress when one is already running. +// The sweep outlives the request, so it runs on a background context, as +// handleTriggerScan's scan does. +func (h *handlers) handleRunDuplicateSweep(w http.ResponseWriter, _ *http.Request) { + // Runs whatever the sweep interval says: the interval paces the automatic + // sweep, and an operator pressing the button has already decided. + started, err := library.TryStartDuplicateSweep( + context.Background(), h.pool, h.logger.With("source", "manual"), h.fingerprintSettings.Get(), + ) + if err != nil { + h.logger.Error("admin: start duplicate sweep", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + if !started { + writeAdminJSONErr(w, http.StatusConflict, "sweep_in_progress") + return + } + writeJSON(w, http.StatusAccepted, map[string]bool{"started": true}) +} + +// handleDismissDuplicateGroup implements POST +// /api/admin/library/duplicates/{id}/dismiss: "these are not duplicates". The +// sweep keeps the dismissal and will not propose that set of tracks again. 404 +// duplicate_group_not_pending when the group was already resolved or is gone. +func (h *handlers) handleDismissDuplicateGroup(w http.ResponseWriter, r *http.Request) { + id, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + n, err := dbq.New(h.pool).DismissDuplicateGroup(r.Context(), id) + if err != nil { + h.logger.Error("admin: dismiss duplicate group", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + if n == 0 { + writeAdminJSONErr(w, http.StatusNotFound, "duplicate_group_not_pending") + return + } + writeJSON(w, http.StatusOK, map[string]string{"status": "dismissed"}) +} + +// mergeDuplicateRequest chooses the copy to keep. An empty survivor_track_id +// keeps the report's proposal. +type mergeDuplicateRequest struct { + SurvivorTrackID string `json:"survivor_track_id"` + Unmonitor bool `json:"unmonitor"` +} + +// mergeDuplicateResponse reports what the merge removed. RemovedPaths are files +// deleted from disk; the operator reads them to know exactly what went. +type mergeDuplicateResponse struct { + SurvivorTrackID string `json:"survivor_track_id"` + RemovedPaths []string `json:"removed_paths"` + LidarrUnmonitorFailed *bool `json:"lidarr_unmonitor_failed,omitempty"` +} + +// mergeRequestBodyLimit bounds the request body. It holds one id and a flag. +const mergeRequestBodyLimit = 1 << 16 + +// handleMergeDuplicateGroup implements POST /api/admin/library/duplicates/{id}/merge +// (#3911): keep one copy, move the others' likes, plays and playlist entries onto +// it, and delete the others' files and rows. +// +// Errors: +// - 409 library_not_writable / 500 file_delete_failed when a file could not be +// removed — nothing was changed (fileRemoveAPIError) +// - 404 duplicate_group_not_pending when the group was already resolved +// - 400 survivor_not_in_group, invalid_id, invalid_body +func (h *handlers) handleMergeDuplicateGroup(w http.ResponseWriter, r *http.Request) { + admin, ok := requireUser(w, r) + if !ok { + return + } + groupID, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + var body mergeDuplicateRequest + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, mergeRequestBodyLimit)).Decode(&body); err != nil && !errors.Is(err, io.EOF) { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_body") + return + } + var survivorID pgtype.UUID // invalid: keep the proposal + if body.SurvivorTrackID != "" { + if survivorID, ok = parseUUID(body.SurvivorTrackID); !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + } + + res, unmonitorFailed, err := h.tracks.MergeDuplicates(r.Context(), groupID, survivorID, admin.ID, body.Unmonitor) + if err != nil { + if apiErr, ok := fileRemoveAPIError(err); ok { + logFileRemoveFailure(h.logger, apiErr, "group_id", uuidToString(groupID)) + writeErr(w, apiErr) + return + } + switch { + case errors.Is(err, library.ErrDuplicateGroupNotPending): + writeAdminJSONErr(w, http.StatusNotFound, "duplicate_group_not_pending") + case errors.Is(err, library.ErrSurvivorNotInGroup): + writeAdminJSONErr(w, http.StatusBadRequest, "survivor_not_in_group") + default: + h.logger.Error("admin: merge duplicate group", "group_id", uuidToString(groupID), "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + } + return + } + + resp := mergeDuplicateResponse{ + SurvivorTrackID: uuidToString(res.Survivor.TrackID), + RemovedPaths: make([]string, 0, len(res.Removed)), + } + for _, c := range res.Removed { + resp.RemovedPaths = append(resp.RemovedPaths, c.FilePath) + } + if body.Unmonitor && unmonitorFailed { + failed := true + resp.LidarrUnmonitorFailed = &failed + } + writeJSON(w, http.StatusOK, resp) +} diff --git a/internal/api/admin_duplicates_test.go b/internal/api/admin_duplicates_test.go new file mode 100644 index 00000000..a1c8a332 --- /dev/null +++ b/internal/api/admin_duplicates_test.go @@ -0,0 +1,65 @@ +package api + +import ( + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +func dupUUID(b byte) pgtype.UUID { + var u pgtype.UUID + u.Bytes[15] = b + u.Valid = true + return u +} + +func dupTS(t time.Time) pgtype.Timestamptz { return pgtype.Timestamptz{Time: t, Valid: true} } + +// Rows arrive one per member, members of a group together. The fold must keep +// groups apart, keep the query's order, and propose each group's survivor from +// its own members only. +func TestFoldDuplicateGroups(t *testing.T) { + older := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + newer := older.Add(48 * time.Hour) + ber := float32(0.04) + rows := []dbq.ListPendingDuplicateGroupMembersRow{ + // Group 1: identical audio, sizes tie, the older copy should be kept. + {GroupID: dupUUID(1), Tier: "exact", DetectedAt: dupTS(newer), TrackID: dupUUID(10), + Title: "WWW", FileFormat: "mp3", FileSize: 6_900_000, DurationMs: 215_400, AddedAt: dupTS(newer), PlayCount: 3}, + {GroupID: dupUUID(1), Tier: "exact", DetectedAt: dupTS(newer), TrackID: dupUUID(11), + Title: "WWW", FileFormat: "mp3", FileSize: 6_900_000, DurationMs: 215_400, AddedAt: dupTS(older), LikeCount: 1}, + // Group 2: the same recording, FLAC against MP3. + {GroupID: dupUUID(2), Tier: "acoustic", WorstBitErrorRate: &ber, DetectedAt: dupTS(older), TrackID: dupUUID(20), + Title: "Lovesick", FileFormat: "mp3", FileSize: 9_000_000, DurationMs: 198_000, AddedAt: dupTS(older)}, + {GroupID: dupUUID(2), Tier: "acoustic", WorstBitErrorRate: &ber, DetectedAt: dupTS(older), TrackID: dupUUID(21), + Title: "Lovesick", FileFormat: "flac", FileSize: 30_000_000, DurationMs: 198_000, AddedAt: dupTS(newer)}, + } + + got := foldDuplicateGroups(rows) + if len(got) != 2 { + t.Fatalf("folded %d groups, want 2", len(got)) + } + + g1, g2 := got[0], got[1] + if g1.ID != uuidToString(dupUUID(1)) || len(g1.Members) != 2 || g1.WorstBitErrorRate != nil { + t.Fatalf("group 1 = %+v, want the exact pair with no score", g1) + } + if g1.SurvivorTrackID != uuidToString(dupUUID(11)) || g1.SurvivorReason != "in the library longest" { + t.Errorf("group 1 survivor = (%s, %q), want the older copy", g1.SurvivorTrackID, g1.SurvivorReason) + } + if g1.Members[0].DurationSec != 215 || g1.Members[0].PlayCount != 3 || g1.Members[1].LikeCount != 1 { + t.Errorf("group 1 members lost their facts: %+v", g1.Members) + } + + if g2.Tier != "acoustic" || g2.WorstBitErrorRate == nil || *g2.WorstBitErrorRate != ber { + t.Fatalf("group 2 = %+v, want the acoustic pair with its score", g2) + } + // Chosen from group 2's own members: a survivor leaking across groups is + // exactly what a wrong fold boundary would produce. + if g2.SurvivorTrackID != uuidToString(dupUUID(21)) || g2.SurvivorReason != "lossless (flac)" { + t.Errorf("group 2 survivor = (%s, %q), want the FLAC copy", g2.SurvivorTrackID, g2.SurvivorReason) + } +} diff --git a/internal/api/admin_fingerprint_settings.go b/internal/api/admin_fingerprint_settings.go new file mode 100644 index 00000000..bf049b68 --- /dev/null +++ b/internal/api/admin_fingerprint_settings.go @@ -0,0 +1,67 @@ +package api + +import ( + "encoding/json" + "errors" + "net/http" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" +) + +// fingerprintSettingsBody is the wire shape for GET and PUT +// /api/admin/library/fingerprint-settings (M400 #3913). The threshold travels as +// the bit-error rate the matcher uses; the card presents it as a match percentage. +type fingerprintSettingsBody struct { + Enabled bool `json:"enabled"` + ChromaprintLengthSec int32 `json:"chromaprint_length_sec"` + AcousticMaxBitErrorRate float64 `json:"acoustic_max_bit_error_rate"` + BackfillConcurrency int32 `json:"backfill_concurrency"` + SweepIntervalHours int32 `json:"sweep_interval_hours"` +} + +func fingerprintSettingsBodyOf(s library.FingerprintSettings) fingerprintSettingsBody { + return fingerprintSettingsBody{ + Enabled: s.Enabled, + ChromaprintLengthSec: s.ChromaprintLengthSec, + AcousticMaxBitErrorRate: s.AcousticMaxBitErrorRate, + BackfillConcurrency: s.BackfillConcurrency, + SweepIntervalHours: s.SweepIntervalHours, + } +} + +// handleGetFingerprintSettings implements GET /api/admin/library/fingerprint-settings. +func (h *handlers) handleGetFingerprintSettings(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, http.StatusOK, fingerprintSettingsBodyOf(h.fingerprintSettings.Get())) +} + +// handleUpdateFingerprintSettings implements PUT /api/admin/library/fingerprint-settings. +// +// A whole-row write. A body that leaves a field out decodes it as zero, which no +// field accepts, so a partial save is refused rather than zeroing what it omitted. +// The saved settings reach the scanner and both workers at once: they share the +// service instance. +func (h *handlers) handleUpdateFingerprintSettings(w http.ResponseWriter, r *http.Request) { + var req fingerprintSettingsBody + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeErr(w, apierror.BadRequest("invalid_body", "malformed JSON")) + return + } + saved, err := h.fingerprintSettings.Set(r.Context(), library.FingerprintSettings{ + Enabled: req.Enabled, + ChromaprintLengthSec: req.ChromaprintLengthSec, + AcousticMaxBitErrorRate: req.AcousticMaxBitErrorRate, + BackfillConcurrency: req.BackfillConcurrency, + SweepIntervalHours: req.SweepIntervalHours, + }) + if err != nil { + // Validation mirrors migration 0061's CHECKs and names the field. + if errors.Is(err, library.ErrFingerprintSettingOutOfRange) { + writeErr(w, apierror.BadRequest("invalid_setting", err.Error())) + return + } + writeErrWithLog(w, h.logger, "admin fingerprint settings: update failed", apierror.Internal(err)) + return + } + writeJSON(w, http.StatusOK, fingerprintSettingsBodyOf(saved)) +} diff --git a/internal/api/admin_fingerprint_settings_test.go b/internal/api/admin_fingerprint_settings_test.go new file mode 100644 index 00000000..056447ea --- /dev/null +++ b/internal/api/admin_fingerprint_settings_test.go @@ -0,0 +1,55 @@ +package api + +import ( + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/library" +) + +func TestGetFingerprintSettings_ServesDefaultsWithoutAService(t *testing.T) { + h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))} + rec := httptest.NewRecorder() + h.handleGetFingerprintSettings(rec, httptest.NewRequest(http.MethodGet, "/api/admin/library/fingerprint-settings", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + var got fingerprintSettingsBody + if err := json.NewDecoder(rec.Body).Decode(&got); err != nil { + t.Fatalf("decode: %v", err) + } + if want := fingerprintSettingsBodyOf(library.DefaultFingerprintSettings); got != want { + t.Fatalf("body = %+v, want the defaults %+v", got, want) + } +} + +func TestUpdateFingerprintSettings_Rejects(t *testing.T) { + h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))} + for name, tc := range map[string]struct { + body string + code string + mentions string + }{ + "a value out of range, naming the field": { + body: `{"enabled":true,"chromaprint_length_sec":5,"acoustic_max_bit_error_rate":0.15,"backfill_concurrency":2,"sweep_interval_hours":1}`, + code: "invalid_setting", + mentions: "chromaprint_length_sec", + }, + // A partial body would otherwise zero every field it left out. + "a body missing fields": {body: `{"enabled":false}`, code: "invalid_setting"}, + "malformed JSON": {body: `{"enabled":`, code: "invalid_body"}, + } { + rec := httptest.NewRecorder() + h.handleUpdateFingerprintSettings(rec, httptest.NewRequest( + http.MethodPut, "/api/admin/library/fingerprint-settings", strings.NewReader(tc.body))) + body := rec.Body.String() + if rec.Code != http.StatusBadRequest || !strings.Contains(body, `"`+tc.code+`"`) || !strings.Contains(body, tc.mentions) { + t.Errorf("%s: status %d body %s; want 400 %s mentioning %q", name, rec.Code, body, tc.code, tc.mentions) + } + } +} diff --git a/internal/api/api.go b/internal/api/api.go index b2e76c2d..6f21467c 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -33,30 +33,31 @@ import ( // Mount attaches /api/* handlers to r. Public endpoints (login) are outside // RequireUser; everything else is gated by the middleware. The events writer // is shared with the Subsonic mount so /rest/scrobble feeds the same store. -func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, tagSettings *tags.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte, netSettings *netsettings.Service, reacqSettings *reacquisition.SettingsService) { +func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, tagSettings *tags.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte, netSettings *netsettings.Service, reacqSettings *reacquisition.SettingsService, fpSettings *library.FingerprintSettingsService) { rng := rand.New(rand.NewSource(rand.Int63())) h := &handlers{ pool: pool, logger: logger, events: events, recCfg: recCfg, - recSettings: recSettings, - rng: rng.Float64, - lidarrCfg: lidarrCfg, - lidarrRequests: lidarrReqs, - lidarrQuarantine: lidarrQuar, - tracks: tracksSvc, - playlists: playlistsSvc, - coverart: coverEnricher, - coverSettings: coverSettings, - tagSettings: tagSettings, - scanner: scanner, - scanCfg: scanCfg, - dataDir: dataDir, - mailer: sender, - eventbus: bus, - playlistScheduler: playlistScheduler, - streamSecret: streamSecret, - netSettings: netSettings, - reacqSettings: reacqSettings, - librarySize: recommendation.NewLibrarySize(nil), + recSettings: recSettings, + rng: rng.Float64, + lidarrCfg: lidarrCfg, + lidarrRequests: lidarrReqs, + lidarrQuarantine: lidarrQuar, + tracks: tracksSvc, + playlists: playlistsSvc, + coverart: coverEnricher, + coverSettings: coverSettings, + tagSettings: tagSettings, + scanner: scanner, + scanCfg: scanCfg, + dataDir: dataDir, + mailer: sender, + eventbus: bus, + playlistScheduler: playlistScheduler, + streamSecret: streamSecret, + netSettings: netSettings, + reacqSettings: reacqSettings, + fingerprintSettings: fpSettings, + librarySize: recommendation.NewLibrarySize(nil), } r.Route("/api", func(api chi.Router) { @@ -216,6 +217,15 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Get("/library/coverage", h.handleGetLibraryCoverage) admin.Get("/library/fingerprints", h.handleGetFingerprintCoverage) + admin.Get("/library/fingerprint-settings", h.handleGetFingerprintSettings) + admin.Put("/library/fingerprint-settings", h.handleUpdateFingerprintSettings) + // Duplicates report (#3912): proposals from the duplicate sweep, a + // trigger to sweep now, dismissal, and the merge (#3911), which deletes + // the removed copies' files after moving their history onto the kept one. + admin.Get("/library/duplicates", h.handleListDuplicates) + admin.Post("/library/duplicates/sweep", h.handleRunDuplicateSweep) + admin.Post("/library/duplicates/{id}/dismiss", h.handleDismissDuplicateGroup) + admin.Post("/library/duplicates/{id}/merge", h.handleMergeDuplicateGroup) admin.Get("/invites", h.handleListInvites) admin.Post("/invites", h.handleCreateInvite) @@ -299,6 +309,10 @@ type handlers struct { // missing files (milestone #290) — grace window, backoff, attempt caps. // Cached in the service, so the admin card reads it without a query. reacqSettings *reacquisition.SettingsService + // fingerprintSettings is the fingerprinting policy (M400 #3913), the same + // instance the scanner and the fingerprint workers read, so a save from the + // admin card reaches them without a restart. Nil serves the defaults. + fingerprintSettings *library.FingerprintSettingsService // netSettings caches the trusted reverse-proxy depth read by the auth // middleware on every request and edited from the admin network card. netSettings *netsettings.Service diff --git a/internal/api/library_test.go b/internal/api/library_test.go index fa2b80d9..81cd1677 100644 --- a/internal/api/library_test.go +++ b/internal/api/library_test.go @@ -465,7 +465,7 @@ func TestRoutesRegisteredInMount(t *testing.T) { r := chi.NewRouter() w := playevents.NewWriter(h.pool, slog.New(slog.NewTextHandler(io.Discard, nil)), 30*time.Minute, 0.5, 30000) - Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.recSettings, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.tagSettings, h.scanner, h.scanCfg, h.dataDir, nil, eventbus.New(), nil, nil, h.netSettings, nil) + Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.recSettings, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.tagSettings, h.scanner, h.scanCfg, h.dataDir, nil, eventbus.New(), nil, nil, h.netSettings, nil, nil) paths := []string{ "/api/artists", @@ -484,6 +484,9 @@ func TestRoutesRegisteredInMount(t *testing.T) { // wired. "/api/admin/library/missing", "/api/admin/library/reacquisition", + "/api/admin/library/fingerprints", + "/api/admin/library/fingerprint-settings", + "/api/admin/library/duplicates", } for _, p := range paths { req := httptest.NewRequest(http.MethodGet, p, nil) diff --git a/internal/audit/audit.go b/internal/audit/audit.go index eb2e8a5b..ef2c5f60 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -55,6 +55,11 @@ const ( // exercised. ActionSessionRevoke Action = "session_revoke" ActionSessionRevokeOthers Action = "session_revoke_others" + + // Duplicate merge (#3911). Irreversible: a copy's file and row are removed + // and its history moved onto the copy kept. The metadata names both, so the + // log can answer "where did that file go" long after the report is gone. + ActionDuplicateMerge Action = "duplicate_merge" ) // Write inserts one audit_log row. metadata is marshaled as JSON; diff --git a/internal/audit/audit_test.go b/internal/audit/audit_test.go index ee413893..84faacd7 100644 --- a/internal/audit/audit_test.go +++ b/internal/audit/audit_test.go @@ -168,6 +168,7 @@ func TestWrite_AllActionConstantsArePersisted(t *testing.T) { audit.ActionTokenRegenerate, audit.ActionForgotPasswordInit, audit.ActionPasswordResetByEmail, + audit.ActionDuplicateMerge, } for _, a := range actions { if err := audit.Write(context.Background(), pool, nilUUID, nilUUID, a, nil); err != nil { diff --git a/internal/db/dbq/duplicates.sql.go b/internal/db/dbq/duplicates.sql.go new file mode 100644 index 00000000..c3dab489 --- /dev/null +++ b/internal/db/dbq/duplicates.sql.go @@ -0,0 +1,457 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: duplicates.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const addDuplicateGroupMember = `-- name: AddDuplicateGroupMember :exec +INSERT INTO duplicate_group_members (group_id, track_id) +VALUES ($1, $2) +ON CONFLICT DO NOTHING +` + +type AddDuplicateGroupMemberParams struct { + GroupID pgtype.UUID + TrackID pgtype.UUID +} + +func (q *Queries) AddDuplicateGroupMember(ctx context.Context, arg AddDuplicateGroupMemberParams) error { + _, err := q.db.Exec(ctx, addDuplicateGroupMember, arg.GroupID, arg.TrackID) + return err +} + +const countPendingDuplicateGroups = `-- name: CountPendingDuplicateGroups :one +SELECT count(*)::bigint + FROM duplicate_groups g + WHERE g.status = 'pending' + AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2 +` + +// Proposals awaiting review. A group left with one member — its other tracks +// deleted since the sweep — is no proposal at all and is not counted; the next +// sweep retires it. +func (q *Queries) CountPendingDuplicateGroups(ctx context.Context) (int64, error) { + row := q.db.QueryRow(ctx, countPendingDuplicateGroups) + var column_1 int64 + err := row.Scan(&column_1) + return column_1, err +} + +const deleteStalePendingDuplicateGroups = `-- name: DeleteStalePendingDuplicateGroups :execrows +DELETE FROM duplicate_groups g + WHERE g.status = 'pending' + AND g.last_seen_sweep_id IS DISTINCT FROM $1 + AND (g.last_seen_sweep_id IS NULL + OR (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = g.last_seen_sweep_id) + < (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = $1)) +` + +// A pending proposal this sweep did not find again no longer describes the +// library: a member was re-fingerprinted, merged away or went missing. Dismissed +// groups are kept regardless — they are the memory of a decision. +// +// Only proposals last confirmed by an EARLIER sweep go. Should two sweeps ever +// overlap (a manual trigger racing the worker), neither may delete what the other +// has just found. +func (q *Queries) DeleteStalePendingDuplicateGroups(ctx context.Context, sweepID pgtype.UUID) (int64, error) { + result, err := q.db.Exec(ctx, deleteStalePendingDuplicateGroups, sweepID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const dismissDuplicateGroup = `-- name: DismissDuplicateGroup :execrows +UPDATE duplicate_groups + SET status = 'dismissed', resolved_at = now() + WHERE id = $1 AND status = 'pending' +` + +// "These are not duplicates." Only a pending group can be dismissed; zero rows +// means it was already resolved or no longer exists. +func (q *Queries) DismissDuplicateGroup(ctx context.Context, id pgtype.UUID) (int64, error) { + result, err := q.db.Exec(ctx, dismissDuplicateGroup, id) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const finishDuplicateSweep = `-- name: FinishDuplicateSweep :exec +UPDATE duplicate_sweeps + SET finished_at = now(), + candidates = $1, + groups_found = $2, + oversize_clusters = $3, + error_message = NULLIF($4::text, '') + WHERE id = $5 +` + +type FinishDuplicateSweepParams struct { + Candidates *int32 + GroupsFound *int32 + OversizeClusters *int32 + ErrorMessage string + ID pgtype.UUID +} + +func (q *Queries) FinishDuplicateSweep(ctx context.Context, arg FinishDuplicateSweepParams) error { + _, err := q.db.Exec(ctx, finishDuplicateSweep, + arg.Candidates, + arg.GroupsFound, + arg.OversizeClusters, + arg.ErrorMessage, + arg.ID, + ) + return err +} + +const getInFlightDuplicateSweep = `-- name: GetInFlightDuplicateSweep :one +SELECT id, started_at + FROM duplicate_sweeps + WHERE finished_at IS NULL + ORDER BY started_at DESC + LIMIT 1 +` + +type GetInFlightDuplicateSweepRow struct { + ID pgtype.UUID + StartedAt pgtype.Timestamptz +} + +// The guard against two sweeps at once: "in flight" is finished_at IS NULL. +func (q *Queries) GetInFlightDuplicateSweep(ctx context.Context) (GetInFlightDuplicateSweepRow, error) { + row := q.db.QueryRow(ctx, getInFlightDuplicateSweep) + var i GetInFlightDuplicateSweepRow + err := row.Scan(&i.ID, &i.StartedAt) + return i, err +} + +const getLatestDuplicateSweep = `-- name: GetLatestDuplicateSweep :one +SELECT id, started_at, finished_at, candidates, groups_found, oversize_clusters, error_message + FROM duplicate_sweeps + ORDER BY started_at DESC + LIMIT 1 +` + +func (q *Queries) GetLatestDuplicateSweep(ctx context.Context) (DuplicateSweep, error) { + row := q.db.QueryRow(ctx, getLatestDuplicateSweep) + var i DuplicateSweep + err := row.Scan( + &i.ID, + &i.StartedAt, + &i.FinishedAt, + &i.Candidates, + &i.GroupsFound, + &i.OversizeClusters, + &i.ErrorMessage, + ) + return i, err +} + +const getLatestFingerprintComputedAt = `-- name: GetLatestFingerprintComputedAt :one +SELECT max(computed_at)::timestamptz AS latest FROM track_fingerprints +` + +// Whether a sweep has anything new to look at: fingerprints written since the +// last sweep started. +func (q *Queries) GetLatestFingerprintComputedAt(ctx context.Context) (pgtype.Timestamptz, error) { + row := q.db.QueryRow(ctx, getLatestFingerprintComputedAt) + var latest pgtype.Timestamptz + err := row.Scan(&latest) + return latest, err +} + +const listDismissedDuplicateMemberSets = `-- name: ListDismissedDuplicateMemberSets :many +SELECT g.id, array_agg(m.track_id ORDER BY m.track_id)::uuid[] AS track_ids + FROM duplicate_groups g + JOIN duplicate_group_members m ON m.group_id = g.id + WHERE g.status = 'dismissed' + GROUP BY g.id +` + +type ListDismissedDuplicateMemberSetsRow struct { + ID pgtype.UUID + TrackIds []pgtype.UUID +} + +// What the operator has already said are not duplicates. A new proposal whose +// every member sat together in one of these is not proposed again. +func (q *Queries) ListDismissedDuplicateMemberSets(ctx context.Context) ([]ListDismissedDuplicateMemberSetsRow, error) { + rows, err := q.db.Query(ctx, listDismissedDuplicateMemberSets) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListDismissedDuplicateMemberSetsRow + for rows.Next() { + var i ListDismissedDuplicateMemberSetsRow + if err := rows.Scan(&i.ID, &i.TrackIds); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listDuplicateCandidates = `-- name: ListDuplicateCandidates :many +SELECT t.id, t.duration_ms, f.audio_stream_sha256, f.chromaprint + FROM tracks t + JOIN track_fingerprints f ON f.track_id = t.id + WHERE t.missing_since IS NULL + AND f.fingerprint_version >= $1 + AND f.chromaprint IS NOT NULL + -- Only chromaprints taken at the current length: prints at two lengths are not + -- comparable, and after a length change the backfill is still re-deriving the + -- rest (#3913). + AND f.chromaprint_length_sec = $2 + AND (t.duration_ms, t.id) > ($3::integer, $4::uuid) + ORDER BY t.duration_ms, t.id + LIMIT $5 +` + +type ListDuplicateCandidatesParams struct { + CurrentVersion int16 + ChromaprintLengthSec int32 + AfterDurationMs int32 + AfterID pgtype.UUID + PageLimit int32 +} + +type ListDuplicateCandidatesRow struct { + ID pgtype.UUID + DurationMs int32 + AudioStreamSha256 []byte + Chromaprint []int32 +} + +// The acoustic tier's input, one page at a time in (duration_ms, id) order so the +// sweep holds only a sliding window of durations. Tracks without a chromaprint +// cannot be compared acoustically and are left out; any exact duplicates among +// them come from ListExactDuplicateHashes. +func (q *Queries) ListDuplicateCandidates(ctx context.Context, arg ListDuplicateCandidatesParams) ([]ListDuplicateCandidatesRow, error) { + rows, err := q.db.Query(ctx, listDuplicateCandidates, + arg.CurrentVersion, + arg.ChromaprintLengthSec, + arg.AfterDurationMs, + arg.AfterID, + arg.PageLimit, + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListDuplicateCandidatesRow + for rows.Next() { + var i ListDuplicateCandidatesRow + if err := rows.Scan( + &i.ID, + &i.DurationMs, + &i.AudioStreamSha256, + &i.Chromaprint, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listExactDuplicateHashes = `-- name: ListExactDuplicateHashes :many +SELECT f.audio_stream_sha256, + array_agg(t.id ORDER BY t.id)::uuid[] AS track_ids + FROM track_fingerprints f + JOIN tracks t ON t.id = f.track_id + WHERE t.missing_since IS NULL + AND f.fingerprint_version >= $1 + AND f.audio_stream_sha256 IS NOT NULL + GROUP BY f.audio_stream_sha256 +HAVING count(*) > 1 +` + +type ListExactDuplicateHashesRow struct { + AudioStreamSha256 []byte + TrackIds []pgtype.UUID +} + +// The exact tier, library-wide in one pass: identical encoded audio shared by +// more than one present track. +func (q *Queries) ListExactDuplicateHashes(ctx context.Context, currentVersion int16) ([]ListExactDuplicateHashesRow, error) { + rows, err := q.db.Query(ctx, listExactDuplicateHashes, currentVersion) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListExactDuplicateHashesRow + for rows.Next() { + var i ListExactDuplicateHashesRow + if err := rows.Scan(&i.AudioStreamSha256, &i.TrackIds); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listPendingDuplicateGroupMembers = `-- name: ListPendingDuplicateGroupMembers :many +WITH page AS ( + SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at + FROM duplicate_groups g + WHERE g.status = 'pending' + AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2 + ORDER BY g.detected_at DESC, g.id + LIMIT $2 OFFSET $1 +) +SELECT p.id AS group_id, + p.tier, + p.worst_bit_error_rate, + p.detected_at, + t.id AS track_id, + t.title, + artists.name AS artist_name, + albums.id AS album_id, + albums.title AS album_title, + t.file_path, + t.file_format, + t.file_size, + t.duration_ms, + t.added_at, + (SELECT count(*) FROM general_likes l WHERE l.track_id = t.id)::bigint AS like_count, + (SELECT count(*) FROM play_events e WHERE e.track_id = t.id)::bigint AS play_count + FROM page p + JOIN duplicate_group_members m ON m.group_id = p.id + JOIN tracks t ON t.id = m.track_id + JOIN albums ON albums.id = t.album_id + JOIN artists ON artists.id = t.artist_id + ORDER BY p.detected_at DESC, p.id, t.id +` + +type ListPendingDuplicateGroupMembersParams struct { + PageOffset int32 + PageLimit int32 +} + +type ListPendingDuplicateGroupMembersRow struct { + GroupID pgtype.UUID + Tier string + WorstBitErrorRate *float32 + DetectedAt pgtype.Timestamptz + TrackID pgtype.UUID + Title string + ArtistName string + AlbumID pgtype.UUID + AlbumTitle string + FilePath string + FileFormat string + FileSize int64 + DurationMs int32 + AddedAt pgtype.Timestamptz + LikeCount int64 + PlayCount int64 +} + +// One page of proposals, newest first, flattened to one row per member so the +// handler folds them without a query per group. What each copy carries — likes +// and plays from every user — is here because it is what the operator weighs +// when deciding which copy to keep. +func (q *Queries) ListPendingDuplicateGroupMembers(ctx context.Context, arg ListPendingDuplicateGroupMembersParams) ([]ListPendingDuplicateGroupMembersRow, error) { + rows, err := q.db.Query(ctx, listPendingDuplicateGroupMembers, arg.PageOffset, arg.PageLimit) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListPendingDuplicateGroupMembersRow + for rows.Next() { + var i ListPendingDuplicateGroupMembersRow + if err := rows.Scan( + &i.GroupID, + &i.Tier, + &i.WorstBitErrorRate, + &i.DetectedAt, + &i.TrackID, + &i.Title, + &i.ArtistName, + &i.AlbumID, + &i.AlbumTitle, + &i.FilePath, + &i.FileFormat, + &i.FileSize, + &i.DurationMs, + &i.AddedAt, + &i.LikeCount, + &i.PlayCount, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const startDuplicateSweep = `-- name: StartDuplicateSweep :one +INSERT INTO duplicate_sweeps DEFAULT VALUES RETURNING id, started_at +` + +type StartDuplicateSweepRow struct { + ID pgtype.UUID + StartedAt pgtype.Timestamptz +} + +func (q *Queries) StartDuplicateSweep(ctx context.Context) (StartDuplicateSweepRow, error) { + row := q.db.QueryRow(ctx, startDuplicateSweep) + var i StartDuplicateSweepRow + err := row.Scan(&i.ID, &i.StartedAt) + return i, err +} + +const upsertDuplicateGroup = `-- name: UpsertDuplicateGroup :one +INSERT INTO duplicate_groups (member_key, tier, worst_bit_error_rate, last_seen_sweep_id) +VALUES ($1, $2, $3, $4) +ON CONFLICT (member_key) DO UPDATE + SET tier = EXCLUDED.tier, + worst_bit_error_rate = EXCLUDED.worst_bit_error_rate, + last_seen_sweep_id = EXCLUDED.last_seen_sweep_id + WHERE duplicate_groups.status = 'pending' +RETURNING id +` + +type UpsertDuplicateGroupParams struct { + MemberKey string + Tier string + WorstBitErrorRate *float32 + SweepID pgtype.UUID +} + +// Proposes a group, or refreshes one already pending. A group already dismissed +// or merged is left exactly as it is: the WHERE on the update makes the conflict +// a no-op, and the caller sees no row. +func (q *Queries) UpsertDuplicateGroup(ctx context.Context, arg UpsertDuplicateGroupParams) (pgtype.UUID, error) { + row := q.db.QueryRow(ctx, upsertDuplicateGroup, + arg.MemberKey, + arg.Tier, + arg.WorstBitErrorRate, + arg.SweepID, + ) + var id pgtype.UUID + err := row.Scan(&id) + return id, err +} diff --git a/internal/db/dbq/fingerprint_settings.sql.go b/internal/db/dbq/fingerprint_settings.sql.go new file mode 100644 index 00000000..ab1ab634 --- /dev/null +++ b/internal/db/dbq/fingerprint_settings.sql.go @@ -0,0 +1,72 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: fingerprint_settings.sql + +package dbq + +import ( + "context" +) + +const getFingerprintSettings = `-- name: GetFingerprintSettings :one +SELECT id, enabled, chromaprint_length_sec, acoustic_max_bit_error_rate, backfill_concurrency, sweep_interval_hours, updated_at FROM fingerprint_settings WHERE id = true +` + +func (q *Queries) GetFingerprintSettings(ctx context.Context) (FingerprintSetting, error) { + row := q.db.QueryRow(ctx, getFingerprintSettings) + var i FingerprintSetting + err := row.Scan( + &i.ID, + &i.Enabled, + &i.ChromaprintLengthSec, + &i.AcousticMaxBitErrorRate, + &i.BackfillConcurrency, + &i.SweepIntervalHours, + &i.UpdatedAt, + ) + return i, err +} + +const updateFingerprintSettings = `-- name: UpdateFingerprintSettings :one +UPDATE fingerprint_settings + SET enabled = $1, + chromaprint_length_sec = $2, + acoustic_max_bit_error_rate = $3, + backfill_concurrency = $4, + sweep_interval_hours = $5, + updated_at = now() + WHERE id = true +RETURNING id, enabled, chromaprint_length_sec, acoustic_max_bit_error_rate, backfill_concurrency, sweep_interval_hours, updated_at +` + +type UpdateFingerprintSettingsParams struct { + Enabled bool + ChromaprintLengthSec int32 + AcousticMaxBitErrorRate float64 + BackfillConcurrency int32 + SweepIntervalHours int32 +} + +// Whole-row write from the admin card; migration 0061's CHECKs are the backstop +// behind the service's own validation. +func (q *Queries) UpdateFingerprintSettings(ctx context.Context, arg UpdateFingerprintSettingsParams) (FingerprintSetting, error) { + row := q.db.QueryRow(ctx, updateFingerprintSettings, + arg.Enabled, + arg.ChromaprintLengthSec, + arg.AcousticMaxBitErrorRate, + arg.BackfillConcurrency, + arg.SweepIntervalHours, + ) + var i FingerprintSetting + err := row.Scan( + &i.ID, + &i.Enabled, + &i.ChromaprintLengthSec, + &i.AcousticMaxBitErrorRate, + &i.BackfillConcurrency, + &i.SweepIntervalHours, + &i.UpdatedAt, + ) + return i, err +} diff --git a/internal/db/dbq/fingerprints.sql.go b/internal/db/dbq/fingerprints.sql.go index ce1bdf03..f92f0cb2 100644 --- a/internal/db/dbq/fingerprints.sql.go +++ b/internal/db/dbq/fingerprints.sql.go @@ -27,20 +27,29 @@ const getFingerprintCoverage = `-- name: GetFingerprintCoverage :one SELECT count(*)::bigint AS total, count(*) FILTER ( WHERE f.fingerprint_version >= $1 + AND f.chromaprint_length_sec = $2 AND f.audio_stream_sha256 IS NOT NULL AND f.chromaprint IS NOT NULL )::bigint AS fingerprinted, count(*) FILTER ( WHERE f.fingerprint_version >= $1 + AND f.chromaprint_length_sec = $2 AND (f.audio_stream_sha256 IS NULL OR f.chromaprint IS NULL) )::bigint AS rejected, count(*) FILTER ( - WHERE f.track_id IS NULL OR f.fingerprint_version < $1 + WHERE f.track_id IS NULL + OR f.fingerprint_version < $1 + OR f.chromaprint_length_sec <> $2 )::bigint AS pending FROM tracks t LEFT JOIN track_fingerprints f ON f.track_id = t.id WHERE t.missing_since IS NULL ` +type GetFingerprintCoverageParams struct { + CurrentVersion int16 + ChromaprintLengthSec int32 +} + type GetFingerprintCoverageRow struct { Total int64 Fingerprinted int64 @@ -49,11 +58,13 @@ type GetFingerprintCoverageRow struct { } // The admin gauge for the backfill. fingerprinted + rejected + pending = total. -// rejected is a row at the current version with a NULL half: a tool ran and -// refused the file, which is settled rather than waiting. Missing tracks are -// excluded, or the gauge could never reach the end. -func (q *Queries) GetFingerprintCoverage(ctx context.Context, currentVersion int16) (GetFingerprintCoverageRow, error) { - row := q.db.QueryRow(ctx, getFingerprintCoverage, currentVersion) +// "Current" means derived by the current method AT the current length: a row at +// another length is pending, because the backfill will re-derive it. rejected is +// a current row with a NULL half: a tool ran and refused the file, which is +// settled rather than waiting. Missing tracks are excluded, or the gauge could +// never reach the end. +func (q *Queries) GetFingerprintCoverage(ctx context.Context, arg GetFingerprintCoverageParams) (GetFingerprintCoverageRow, error) { + row := q.db.QueryRow(ctx, getFingerprintCoverage, arg.CurrentVersion, arg.ChromaprintLengthSec) var i GetFingerprintCoverageRow err := row.Scan( &i.Total, @@ -69,16 +80,21 @@ SELECT t.id, t.file_path FROM tracks t LEFT JOIN track_fingerprints f ON f.track_id = t.id WHERE t.missing_since IS NULL - AND (f.track_id IS NULL OR f.fingerprint_version < $1) - AND t.id > $2 + -- A row taken at another length is as stale as one from an older method: + -- chromaprints at two lengths cannot be compared (#3913). + AND (f.track_id IS NULL + OR f.fingerprint_version < $1 + OR f.chromaprint_length_sec <> $2) + AND t.id > $3 ORDER BY t.id - LIMIT $3 + LIMIT $4 ` type ListTracksNeedingFingerprintParams struct { - CurrentVersion int16 - AfterID pgtype.UUID - BatchLimit int32 + CurrentVersion int16 + ChromaprintLengthSec int32 + AfterID pgtype.UUID + BatchLimit int32 } type ListTracksNeedingFingerprintRow struct { @@ -93,7 +109,12 @@ type ListTracksNeedingFingerprintRow struct { // and retried in a tight loop. Missing tracks are skipped — there is no file to // read. func (q *Queries) ListTracksNeedingFingerprint(ctx context.Context, arg ListTracksNeedingFingerprintParams) ([]ListTracksNeedingFingerprintRow, error) { - rows, err := q.db.Query(ctx, listTracksNeedingFingerprint, arg.CurrentVersion, arg.AfterID, arg.BatchLimit) + rows, err := q.db.Query(ctx, listTracksNeedingFingerprint, + arg.CurrentVersion, + arg.ChromaprintLengthSec, + arg.AfterID, + arg.BatchLimit, + ) if err != nil { return nil, err } @@ -114,23 +135,25 @@ func (q *Queries) ListTracksNeedingFingerprint(ctx context.Context, arg ListTrac const upsertTrackFingerprint = `-- name: UpsertTrackFingerprint :exec INSERT INTO track_fingerprints ( - track_id, audio_stream_sha256, chromaprint, fingerprint_version + track_id, audio_stream_sha256, chromaprint, fingerprint_version, chromaprint_length_sec ) VALUES ( $1, $2, $3, - $4 + $4, $5 ) ON CONFLICT (track_id) DO UPDATE SET - audio_stream_sha256 = EXCLUDED.audio_stream_sha256, - chromaprint = EXCLUDED.chromaprint, - fingerprint_version = EXCLUDED.fingerprint_version, - computed_at = now() + audio_stream_sha256 = EXCLUDED.audio_stream_sha256, + chromaprint = EXCLUDED.chromaprint, + fingerprint_version = EXCLUDED.fingerprint_version, + chromaprint_length_sec = EXCLUDED.chromaprint_length_sec, + computed_at = now() ` type UpsertTrackFingerprintParams struct { - TrackID pgtype.UUID - AudioStreamSha256 []byte - Chromaprint []int32 - FingerprintVersion int16 + TrackID pgtype.UUID + AudioStreamSha256 []byte + Chromaprint []int32 + FingerprintVersion int16 + ChromaprintLengthSec int32 } // Written whenever a track's fingerprint is derived: by the scan when a file is @@ -143,6 +166,7 @@ func (q *Queries) UpsertTrackFingerprint(ctx context.Context, arg UpsertTrackFin arg.AudioStreamSha256, arg.Chromaprint, arg.FingerprintVersion, + arg.ChromaprintLengthSec, ) return err } diff --git a/internal/db/dbq/merge.sql.go b/internal/db/dbq/merge.sql.go new file mode 100644 index 00000000..95240836 --- /dev/null +++ b/internal/db/dbq/merge.sql.go @@ -0,0 +1,339 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: merge.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const listDuplicateGroupMergeMembers = `-- name: ListDuplicateGroupMergeMembers :many +SELECT t.id, t.file_path, t.file_format, t.file_size, t.added_at, t.album_id, + t.mbid, albums.mbid AS album_mbid + FROM duplicate_group_members m + JOIN tracks t ON t.id = m.track_id + JOIN albums ON albums.id = t.album_id + WHERE m.group_id = $1 + ORDER BY t.id +` + +type ListDuplicateGroupMergeMembersRow struct { + ID pgtype.UUID + FilePath string + FileFormat string + FileSize int64 + AddedAt pgtype.Timestamptz + AlbumID pgtype.UUID + Mbid *string + AlbumMbid *string +} + +func (q *Queries) ListDuplicateGroupMergeMembers(ctx context.Context, groupID pgtype.UUID) ([]ListDuplicateGroupMergeMembersRow, error) { + rows, err := q.db.Query(ctx, listDuplicateGroupMergeMembers, groupID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListDuplicateGroupMergeMembersRow + for rows.Next() { + var i ListDuplicateGroupMergeMembersRow + if err := rows.Scan( + &i.ID, + &i.FilePath, + &i.FileFormat, + &i.FileSize, + &i.AddedAt, + &i.AlbumID, + &i.Mbid, + &i.AlbumMbid, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const lockDuplicateGroupForMerge = `-- name: LockDuplicateGroupForMerge :one + +SELECT id, tier, status + FROM duplicate_groups + WHERE id = $1 + FOR UPDATE +` + +type LockDuplicateGroupForMergeRow struct { + ID pgtype.UUID + Tier string + Status string +} + +// Duplicate merge (Scribe #3911). Every statement here runs inside the one +// transaction library.MergeDuplicateGroup opens, after the removed copy's file +// is already gone. The loser's own track row is deleted last with DeleteTrack; +// what these do is move everything it carries onto the survivor first, so that +// delete's CASCADE finds nothing left to destroy. +// Locks the group for the rest of the transaction, so two merges of one group +// cannot run at once. +func (q *Queries) LockDuplicateGroupForMerge(ctx context.Context, id pgtype.UUID) (LockDuplicateGroupForMergeRow, error) { + row := q.db.QueryRow(ctx, lockDuplicateGroupForMerge, id) + var i LockDuplicateGroupForMergeRow + err := row.Scan(&i.ID, &i.Tier, &i.Status) + return i, err +} + +const markDuplicateGroupMerged = `-- name: MarkDuplicateGroupMerged :execrows +UPDATE duplicate_groups + SET status = 'merged', resolved_at = now() + WHERE id = $1 AND status = 'pending' +` + +func (q *Queries) MarkDuplicateGroupMerged(ctx context.Context, id pgtype.UUID) (int64, error) { + result, err := q.db.Exec(ctx, markDuplicateGroupMerged, id) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const mergeCopyGeneralLikes = `-- name: MergeCopyGeneralLikes :many + +INSERT INTO general_likes (user_id, track_id, liked_at) +SELECT user_id, $1::uuid, liked_at + FROM general_likes + WHERE track_id = $2::uuid +ON CONFLICT (user_id, track_id) DO UPDATE + SET liked_at = LEAST(general_likes.liked_at, EXCLUDED.liked_at) +RETURNING user_id +` + +type MergeCopyGeneralLikesParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +// Collision-safe merges: a unique key includes track_id, so the survivor may +// already hold a matching row. Copy what it lacks; DeleteTrack's CASCADE then +// removes the loser's originals. +// One like per user. A user who liked both copies keeps a single like, dated to +// the earlier of the two. +func (q *Queries) MergeCopyGeneralLikes(ctx context.Context, arg MergeCopyGeneralLikesParams) ([]pgtype.UUID, error) { + rows, err := q.db.Query(ctx, mergeCopyGeneralLikes, arg.SurvivorID, arg.LoserID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []pgtype.UUID + for rows.Next() { + var user_id pgtype.UUID + if err := rows.Scan(&user_id); err != nil { + return nil, err + } + items = append(items, user_id) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const mergeCopyTrackSimilarity = `-- name: MergeCopyTrackSimilarity :execrows +INSERT INTO track_similarity (track_a_id, track_b_id, score, source, fetched_at) +SELECT CASE WHEN track_a_id = $1::uuid THEN $2::uuid ELSE track_a_id END, + CASE WHEN track_b_id = $1::uuid THEN $2::uuid ELSE track_b_id END, + score, source, fetched_at + FROM track_similarity + WHERE (track_a_id = $1::uuid OR track_b_id = $1::uuid) + AND (CASE WHEN track_a_id = $1::uuid THEN $2::uuid ELSE track_a_id END) + <> (CASE WHEN track_b_id = $1::uuid THEN $2::uuid ELSE track_b_id END) +ON CONFLICT (track_a_id, track_b_id, source) DO NOTHING +` + +type MergeCopyTrackSimilarityParams struct { + LoserID pgtype.UUID + SurvivorID pgtype.UUID +} + +// Rewrites the loser to the survivor on either side of an edge. An edge between +// the two copies would become a track similar to itself — the table forbids +// that, and it means nothing — so it is dropped. An edge the survivor already +// has from the same source is kept as it is. +func (q *Queries) MergeCopyTrackSimilarity(ctx context.Context, arg MergeCopyTrackSimilarityParams) (int64, error) { + result, err := q.db.Exec(ctx, mergeCopyTrackSimilarity, arg.LoserID, arg.SurvivorID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const mergeCopyTrackTags = `-- name: MergeCopyTrackTags :execrows +INSERT INTO track_tags (track_id, tag, weight) +SELECT $1::uuid, tag, weight + FROM track_tags + WHERE track_id = $2::uuid +ON CONFLICT (track_id, tag) DO NOTHING +` + +type MergeCopyTrackTagsParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +func (q *Queries) MergeCopyTrackTags(ctx context.Context, arg MergeCopyTrackTagsParams) (int64, error) { + result, err := q.db.Exec(ctx, mergeCopyTrackTags, arg.SurvivorID, arg.LoserID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const mergeInheritTrackMbid = `-- name: MergeInheritTrackMbid :exec +UPDATE tracks AS survivor + SET mbid = loser.mbid + FROM tracks AS loser + WHERE survivor.id = $1::uuid + AND loser.id = $2::uuid + AND survivor.mbid IS NULL + AND loser.mbid IS NOT NULL +` + +type MergeInheritTrackMbidParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +// A recording MBID is what the similarity pipeline keys on. If only the removed +// copy carried one, the survivor takes it rather than going dark to similarity. +func (q *Queries) MergeInheritTrackMbid(ctx context.Context, arg MergeInheritTrackMbidParams) error { + _, err := q.db.Exec(ctx, mergeInheritTrackMbid, arg.SurvivorID, arg.LoserID) + return err +} + +const mergeRepointContextualLikes = `-- name: MergeRepointContextualLikes :execrows +UPDATE contextual_likes SET track_id = $1::uuid WHERE track_id = $2::uuid +` + +type MergeRepointContextualLikesParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +func (q *Queries) MergeRepointContextualLikes(ctx context.Context, arg MergeRepointContextualLikesParams) (int64, error) { + result, err := q.db.Exec(ctx, mergeRepointContextualLikes, arg.SurvivorID, arg.LoserID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const mergeRepointLidarrRequests = `-- name: MergeRepointLidarrRequests :execrows +UPDATE lidarr_requests SET matched_track_id = $1::uuid + WHERE matched_track_id = $2::uuid +` + +type MergeRepointLidarrRequestsParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +func (q *Queries) MergeRepointLidarrRequests(ctx context.Context, arg MergeRepointLidarrRequestsParams) (int64, error) { + result, err := q.db.Exec(ctx, mergeRepointLidarrRequests, arg.SurvivorID, arg.LoserID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const mergeRepointPlayEvents = `-- name: MergeRepointPlayEvents :execrows + +UPDATE play_events SET track_id = $1::uuid WHERE track_id = $2::uuid +` + +type MergeRepointPlayEventsParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +// Plain repoints: no unique key involves track_id, so moving rows cannot collide. +func (q *Queries) MergeRepointPlayEvents(ctx context.Context, arg MergeRepointPlayEventsParams) (int64, error) { + result, err := q.db.Exec(ctx, mergeRepointPlayEvents, arg.SurvivorID, arg.LoserID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const mergeRepointPlaybackErrors = `-- name: MergeRepointPlaybackErrors :execrows +UPDATE playback_errors SET track_id = $1::uuid WHERE track_id = $2::uuid +` + +type MergeRepointPlaybackErrorsParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +func (q *Queries) MergeRepointPlaybackErrors(ctx context.Context, arg MergeRepointPlaybackErrorsParams) (int64, error) { + result, err := q.db.Exec(ctx, mergeRepointPlaybackErrors, arg.SurvivorID, arg.LoserID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const mergeRepointPlaylistTracks = `-- name: MergeRepointPlaylistTracks :many +UPDATE playlist_tracks SET track_id = $1::uuid + WHERE track_id = $2::uuid +RETURNING playlist_id +` + +type MergeRepointPlaylistTracksParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +// playlist_tracks is keyed by (playlist_id, position), so repointing keeps every +// entry exactly where it was. A playlist that held both copies simply holds the +// survivor twice — the user put two entries there, and both stay. +func (q *Queries) MergeRepointPlaylistTracks(ctx context.Context, arg MergeRepointPlaylistTracksParams) ([]pgtype.UUID, error) { + rows, err := q.db.Query(ctx, mergeRepointPlaylistTracks, arg.SurvivorID, arg.LoserID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []pgtype.UUID + for rows.Next() { + var playlist_id pgtype.UUID + if err := rows.Scan(&playlist_id); err != nil { + return nil, err + } + items = append(items, playlist_id) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const mergeRepointSkipEvents = `-- name: MergeRepointSkipEvents :execrows +UPDATE skip_events SET track_id = $1::uuid WHERE track_id = $2::uuid +` + +type MergeRepointSkipEventsParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +func (q *Queries) MergeRepointSkipEvents(ctx context.Context, arg MergeRepointSkipEventsParams) (int64, error) { + result, err := q.db.Exec(ctx, mergeRepointSkipEvents, arg.SurvivorID, arg.LoserID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 07ca6f75..17a9c89d 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -297,6 +297,42 @@ type DiscoverTuning struct { UpdatedAt pgtype.Timestamptz } +type DuplicateGroup struct { + ID pgtype.UUID + MemberKey string + Tier string + WorstBitErrorRate *float32 + Status string + DetectedAt pgtype.Timestamptz + LastSeenSweepID pgtype.UUID + ResolvedAt pgtype.Timestamptz +} + +type DuplicateGroupMember struct { + GroupID pgtype.UUID + TrackID pgtype.UUID +} + +type DuplicateSweep struct { + ID pgtype.UUID + StartedAt pgtype.Timestamptz + FinishedAt pgtype.Timestamptz + Candidates *int32 + GroupsFound *int32 + OversizeClusters *int32 + ErrorMessage *string +} + +type FingerprintSetting struct { + ID bool + Enabled bool + ChromaprintLengthSec int32 + AcousticMaxBitErrorRate float64 + BackfillConcurrency int32 + SweepIntervalHours int32 + UpdatedAt pgtype.Timestamptz +} + type GeneralLike struct { UserID pgtype.UUID TrackID pgtype.UUID @@ -668,11 +704,12 @@ type Track struct { } type TrackFingerprint struct { - TrackID pgtype.UUID - AudioStreamSha256 []byte - Chromaprint []int32 - FingerprintVersion int16 - ComputedAt pgtype.Timestamptz + TrackID pgtype.UUID + AudioStreamSha256 []byte + Chromaprint []int32 + FingerprintVersion int16 + ComputedAt pgtype.Timestamptz + ChromaprintLengthSec int32 } type TrackSimilarity struct { diff --git a/internal/db/dbq/tracks.sql.go b/internal/db/dbq/tracks.sql.go index 3784cfd4..4cfe2964 100644 --- a/internal/db/dbq/tracks.sql.go +++ b/internal/db/dbq/tracks.sql.go @@ -148,39 +148,36 @@ func (q *Queries) DeleteTrack(ctx context.Context, id pgtype.UUID) (DeleteTrackR return i, err } -const findMissingTrackByFingerprint = `-- name: FindMissingTrackByFingerprint :many -SELECT id, file_path FROM tracks - WHERE missing_since IS NOT NULL - AND file_size = $1 - AND duration_ms = $2 +const findMissingTrackByAudioHash = `-- name: FindMissingTrackByAudioHash :many +SELECT t.id, t.file_path + FROM tracks t + JOIN track_fingerprints f ON f.track_id = t.id + WHERE t.missing_since IS NOT NULL + AND f.audio_stream_sha256 = $1 LIMIT 2 ` -type FindMissingTrackByFingerprintParams struct { - FileSize int64 - DurationMs int32 -} - -type FindMissingTrackByFingerprintRow struct { +type FindMissingTrackByAudioHashRow struct { ID pgtype.UUID FilePath string } -// Move detection fallback for files with no MBID (#2528). Exact byte size AND -// exact decoded duration is a strong pair: a plain move or rename preserves -// both, while a re-encode changes at least one — and a re-encode genuinely is a -// different file, so failing to match there is correct rather than a gap. +// Move detection fallback for files with no MBID (#2528, #3914). The audio stream +// hash identifies the encoded audio itself, so it survives a rename, a move and a +// retag — anything short of a re-encode. It replaced (file_size, duration_ms), +// which could pair two unrelated files that happened to share a byte count and a +// duration, and missed a file retagged in place, whose size changes. // // Same missing-only constraint and same LIMIT 2 rationale as the MBID variant. -func (q *Queries) FindMissingTrackByFingerprint(ctx context.Context, arg FindMissingTrackByFingerprintParams) ([]FindMissingTrackByFingerprintRow, error) { - rows, err := q.db.Query(ctx, findMissingTrackByFingerprint, arg.FileSize, arg.DurationMs) +func (q *Queries) FindMissingTrackByAudioHash(ctx context.Context, audioStreamSha256 []byte) ([]FindMissingTrackByAudioHashRow, error) { + rows, err := q.db.Query(ctx, findMissingTrackByAudioHash, audioStreamSha256) if err != nil { return nil, err } defer rows.Close() - var items []FindMissingTrackByFingerprintRow + var items []FindMissingTrackByAudioHashRow for rows.Next() { - var i FindMissingTrackByFingerprintRow + var i FindMissingTrackByAudioHashRow if err := rows.Scan(&i.ID, &i.FilePath); err != nil { return nil, err } diff --git a/internal/db/migrations/0059_duplicate_groups.down.sql b/internal/db/migrations/0059_duplicate_groups.down.sql new file mode 100644 index 00000000..1d37bf4c --- /dev/null +++ b/internal/db/migrations/0059_duplicate_groups.down.sql @@ -0,0 +1,4 @@ +DROP INDEX IF EXISTS tracks_duration_id_idx; +DROP TABLE duplicate_group_members; +DROP TABLE duplicate_groups; +DROP TABLE duplicate_sweeps; diff --git a/internal/db/migrations/0059_duplicate_groups.up.sql b/internal/db/migrations/0059_duplicate_groups.up.sql new file mode 100644 index 00000000..17a112b5 --- /dev/null +++ b/internal/db/migrations/0059_duplicate_groups.up.sql @@ -0,0 +1,54 @@ +-- 0059_duplicate_groups.up.sql — proposed duplicates and the sweeps that find +-- them (Scribe milestone #400: #3910). +-- +-- The sweep compares fingerprints (track_fingerprints, 0058) and proposes groups +-- of tracks that hold one recording. Nothing here merges anything: a group is a +-- proposal the operator reviews, and the merge (#3911) is a separate act. + +-- One row per sweep. Lets the report tell "the sweep has never run" apart from +-- "it ran and found nothing", and gives the in-flight guard something to check, +-- the same way scan_runs does for the library scan. +CREATE TABLE duplicate_sweeps ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + started_at timestamptz NOT NULL DEFAULT now(), + finished_at timestamptz, + candidates integer, + groups_found integer, + oversize_clusters integer, + error_message text +); +CREATE INDEX duplicate_sweeps_started_at_idx ON duplicate_sweeps (started_at DESC); + +CREATE TABLE duplicate_groups ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + -- The group's identity: its member track ids, sorted and joined. A sweep + -- that finds the same tracks again updates this row rather than proposing + -- them twice, and a dismissal stays attached to the set it was made about. + member_key text NOT NULL UNIQUE, + -- Rule 36: a new value for either CHECK swaps the constraint in the same + -- migration. + tier text NOT NULL CHECK (tier IN ('exact', 'acoustic')), + -- Largest disagreement between any two members; NULL for exact groups, + -- which have no score. + worst_bit_error_rate real, + status text NOT NULL DEFAULT 'pending' + CHECK (status IN ('pending', 'dismissed', 'merged')), + detected_at timestamptz NOT NULL DEFAULT now(), + last_seen_sweep_id uuid REFERENCES duplicate_sweeps (id) ON DELETE SET NULL, + resolved_at timestamptz +); +CREATE INDEX duplicate_groups_status_idx ON duplicate_groups (status); + +CREATE TABLE duplicate_group_members ( + group_id uuid NOT NULL REFERENCES duplicate_groups (id) ON DELETE CASCADE, + -- CASCADE is right here: a track that genuinely leaves the library has no + -- place in a proposal about its duplicates. + track_id uuid NOT NULL REFERENCES tracks (id) ON DELETE CASCADE, + PRIMARY KEY (group_id, track_id) +); +CREATE INDEX duplicate_group_members_track_idx ON duplicate_group_members (track_id); + +-- The sweep streams candidates in (duration_ms, id) order, keyset-paged, so it +-- only ever holds a few seconds' worth of durations in memory. Without this each +-- page would sort the whole library again. +CREATE INDEX tracks_duration_id_idx ON tracks (duration_ms, id); diff --git a/internal/db/migrations/0060_play_events_track_index.down.sql b/internal/db/migrations/0060_play_events_track_index.down.sql new file mode 100644 index 00000000..af749a3f --- /dev/null +++ b/internal/db/migrations/0060_play_events_track_index.down.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS play_events_track_idx; diff --git a/internal/db/migrations/0060_play_events_track_index.up.sql b/internal/db/migrations/0060_play_events_track_index.up.sql new file mode 100644 index 00000000..35036982 --- /dev/null +++ b/internal/db/migrations/0060_play_events_track_index.up.sql @@ -0,0 +1,8 @@ +-- 0060_play_events_track_index.up.sql — play_events by track (Scribe #3912, #3911). +-- +-- play_events is indexed by (user_id, started_at) and (user_id, track_id), both +-- led by user. Nothing reached it by track alone until the duplicates report, +-- which shows each copy's play count — a scan of the whole table per copy — and +-- the merge (#3911), which repoints a duplicate's play history onto the copy +-- being kept. Both ask "every play of this track", whoever played it. +CREATE INDEX play_events_track_idx ON play_events (track_id); diff --git a/internal/db/migrations/0061_fingerprint_settings.down.sql b/internal/db/migrations/0061_fingerprint_settings.down.sql new file mode 100644 index 00000000..31d6d9cc --- /dev/null +++ b/internal/db/migrations/0061_fingerprint_settings.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE track_fingerprints DROP COLUMN chromaprint_length_sec; +DROP TABLE fingerprint_settings; diff --git a/internal/db/migrations/0061_fingerprint_settings.up.sql b/internal/db/migrations/0061_fingerprint_settings.up.sql new file mode 100644 index 00000000..021f85af --- /dev/null +++ b/internal/db/migrations/0061_fingerprint_settings.up.sql @@ -0,0 +1,50 @@ +-- 0061_fingerprint_settings.up.sql — fingerprinting's knobs, in admin Settings +-- (Scribe #3913, milestone #400). Rule 25: anything an operator might tune is a +-- database row, changed without a restart. Singleton in the style of +-- reacquisition_settings (0056). +CREATE TABLE fingerprint_settings ( + id boolean PRIMARY KEY DEFAULT true, + + -- Fingerprinting new files, the backfill, and the duplicate sweep. Off stops + -- the decode work entirely — the reason to turn it off is a slow NAS, and + -- that is the operator's call. On by default: a library that cannot tell its + -- duplicates apart is what milestone #400 exists to end. + enabled boolean NOT NULL DEFAULT true, + + -- Seconds of audio fpcalc fingerprints. Chromaprints taken at different + -- lengths cannot be compared, which is why track_fingerprints records the + -- length each row was taken at (below): change this and every chromaprint is + -- re-derived, and until then only rows at the new length are compared. + chromaprint_length_sec integer NOT NULL DEFAULT 120, + + -- The most disagreement two aligned fingerprints may show and still be + -- proposed as one recording. Unrelated audio sits near 0.5, so the ceiling + -- stays well clear of it. + acoustic_max_bit_error_rate double precision NOT NULL DEFAULT 0.15, + + -- Files the backfill decodes at once. Decoding competes with playback + -- transcoding for CPU and with streaming for the mount. + backfill_concurrency integer NOT NULL DEFAULT 2, + + -- The least time between duplicate sweeps. A sweep still runs only when + -- fingerprints have changed since the last one. + sweep_interval_hours integer NOT NULL DEFAULT 1, + + -- When the settings were last saved. A new threshold or length can change + -- what a sweep finds, so a save makes a sweep due. + updated_at timestamptz NOT NULL DEFAULT now(), + + CONSTRAINT fingerprint_settings_singleton CHECK (id = true), + CONSTRAINT fingerprint_settings_length_range + CHECK (chromaprint_length_sec >= 30 AND chromaprint_length_sec <= 600), + CONSTRAINT fingerprint_settings_threshold_range + CHECK (acoustic_max_bit_error_rate >= 0.01 AND acoustic_max_bit_error_rate <= 0.35), + CONSTRAINT fingerprint_settings_concurrency_range + CHECK (backfill_concurrency >= 1 AND backfill_concurrency <= 8), + CONSTRAINT fingerprint_settings_sweep_interval_range + CHECK (sweep_interval_hours >= 1 AND sweep_interval_hours <= 168) +); +INSERT INTO fingerprint_settings (id) VALUES (true) ON CONFLICT (id) DO NOTHING; + +-- Every row written so far was taken at fpcalc's default length. +ALTER TABLE track_fingerprints ADD COLUMN chromaprint_length_sec integer NOT NULL DEFAULT 120; diff --git a/internal/db/queries/duplicates.sql b/internal/db/queries/duplicates.sql new file mode 100644 index 00000000..9150ccc5 --- /dev/null +++ b/internal/db/queries/duplicates.sql @@ -0,0 +1,156 @@ +-- name: StartDuplicateSweep :one +INSERT INTO duplicate_sweeps DEFAULT VALUES RETURNING id, started_at; + +-- name: FinishDuplicateSweep :exec +UPDATE duplicate_sweeps + SET finished_at = now(), + candidates = sqlc.arg(candidates), + groups_found = sqlc.arg(groups_found), + oversize_clusters = sqlc.arg(oversize_clusters), + error_message = NULLIF(sqlc.arg(error_message)::text, '') + WHERE id = sqlc.arg(id); + +-- name: GetInFlightDuplicateSweep :one +-- The guard against two sweeps at once: "in flight" is finished_at IS NULL. +SELECT id, started_at + FROM duplicate_sweeps + WHERE finished_at IS NULL + ORDER BY started_at DESC + LIMIT 1; + +-- name: GetLatestDuplicateSweep :one +SELECT id, started_at, finished_at, candidates, groups_found, oversize_clusters, error_message + FROM duplicate_sweeps + ORDER BY started_at DESC + LIMIT 1; + +-- name: GetLatestFingerprintComputedAt :one +-- Whether a sweep has anything new to look at: fingerprints written since the +-- last sweep started. +SELECT max(computed_at)::timestamptz AS latest FROM track_fingerprints; + +-- name: ListExactDuplicateHashes :many +-- The exact tier, library-wide in one pass: identical encoded audio shared by +-- more than one present track. +SELECT f.audio_stream_sha256, + array_agg(t.id ORDER BY t.id)::uuid[] AS track_ids + FROM track_fingerprints f + JOIN tracks t ON t.id = f.track_id + WHERE t.missing_since IS NULL + AND f.fingerprint_version >= sqlc.arg(current_version) + AND f.audio_stream_sha256 IS NOT NULL + GROUP BY f.audio_stream_sha256 +HAVING count(*) > 1; + +-- name: ListDuplicateCandidates :many +-- The acoustic tier's input, one page at a time in (duration_ms, id) order so the +-- sweep holds only a sliding window of durations. Tracks without a chromaprint +-- cannot be compared acoustically and are left out; any exact duplicates among +-- them come from ListExactDuplicateHashes. +SELECT t.id, t.duration_ms, f.audio_stream_sha256, f.chromaprint + FROM tracks t + JOIN track_fingerprints f ON f.track_id = t.id + WHERE t.missing_since IS NULL + AND f.fingerprint_version >= sqlc.arg(current_version) + AND f.chromaprint IS NOT NULL + -- Only chromaprints taken at the current length: prints at two lengths are not + -- comparable, and after a length change the backfill is still re-deriving the + -- rest (#3913). + AND f.chromaprint_length_sec = sqlc.arg(chromaprint_length_sec) + AND (t.duration_ms, t.id) > (sqlc.arg(after_duration_ms)::integer, sqlc.arg(after_id)::uuid) + ORDER BY t.duration_ms, t.id + LIMIT sqlc.arg(page_limit); + +-- name: ListDismissedDuplicateMemberSets :many +-- What the operator has already said are not duplicates. A new proposal whose +-- every member sat together in one of these is not proposed again. +SELECT g.id, array_agg(m.track_id ORDER BY m.track_id)::uuid[] AS track_ids + FROM duplicate_groups g + JOIN duplicate_group_members m ON m.group_id = g.id + WHERE g.status = 'dismissed' + GROUP BY g.id; + +-- name: UpsertDuplicateGroup :one +-- Proposes a group, or refreshes one already pending. A group already dismissed +-- or merged is left exactly as it is: the WHERE on the update makes the conflict +-- a no-op, and the caller sees no row. +INSERT INTO duplicate_groups (member_key, tier, worst_bit_error_rate, last_seen_sweep_id) +VALUES (sqlc.arg(member_key), sqlc.arg(tier), sqlc.narg(worst_bit_error_rate), sqlc.arg(sweep_id)) +ON CONFLICT (member_key) DO UPDATE + SET tier = EXCLUDED.tier, + worst_bit_error_rate = EXCLUDED.worst_bit_error_rate, + last_seen_sweep_id = EXCLUDED.last_seen_sweep_id + WHERE duplicate_groups.status = 'pending' +RETURNING id; + +-- name: AddDuplicateGroupMember :exec +INSERT INTO duplicate_group_members (group_id, track_id) +VALUES (sqlc.arg(group_id), sqlc.arg(track_id)) +ON CONFLICT DO NOTHING; + +-- name: DeleteStalePendingDuplicateGroups :execrows +-- A pending proposal this sweep did not find again no longer describes the +-- library: a member was re-fingerprinted, merged away or went missing. Dismissed +-- groups are kept regardless — they are the memory of a decision. +-- +-- Only proposals last confirmed by an EARLIER sweep go. Should two sweeps ever +-- overlap (a manual trigger racing the worker), neither may delete what the other +-- has just found. +DELETE FROM duplicate_groups g + WHERE g.status = 'pending' + AND g.last_seen_sweep_id IS DISTINCT FROM sqlc.arg(sweep_id) + AND (g.last_seen_sweep_id IS NULL + OR (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = g.last_seen_sweep_id) + < (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = sqlc.arg(sweep_id))); + +-- name: CountPendingDuplicateGroups :one +-- Proposals awaiting review. A group left with one member — its other tracks +-- deleted since the sweep — is no proposal at all and is not counted; the next +-- sweep retires it. +SELECT count(*)::bigint + FROM duplicate_groups g + WHERE g.status = 'pending' + AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2; + +-- name: ListPendingDuplicateGroupMembers :many +-- One page of proposals, newest first, flattened to one row per member so the +-- handler folds them without a query per group. What each copy carries — likes +-- and plays from every user — is here because it is what the operator weighs +-- when deciding which copy to keep. +WITH page AS ( + SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at + FROM duplicate_groups g + WHERE g.status = 'pending' + AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2 + ORDER BY g.detected_at DESC, g.id + LIMIT sqlc.arg(page_limit) OFFSET sqlc.arg(page_offset) +) +SELECT p.id AS group_id, + p.tier, + p.worst_bit_error_rate, + p.detected_at, + t.id AS track_id, + t.title, + artists.name AS artist_name, + albums.id AS album_id, + albums.title AS album_title, + t.file_path, + t.file_format, + t.file_size, + t.duration_ms, + t.added_at, + (SELECT count(*) FROM general_likes l WHERE l.track_id = t.id)::bigint AS like_count, + (SELECT count(*) FROM play_events e WHERE e.track_id = t.id)::bigint AS play_count + FROM page p + JOIN duplicate_group_members m ON m.group_id = p.id + JOIN tracks t ON t.id = m.track_id + JOIN albums ON albums.id = t.album_id + JOIN artists ON artists.id = t.artist_id + ORDER BY p.detected_at DESC, p.id, t.id; + +-- name: DismissDuplicateGroup :execrows +-- "These are not duplicates." Only a pending group can be dismissed; zero rows +-- means it was already resolved or no longer exists. +UPDATE duplicate_groups + SET status = 'dismissed', resolved_at = now() + WHERE id = sqlc.arg(id) AND status = 'pending'; diff --git a/internal/db/queries/fingerprint_settings.sql b/internal/db/queries/fingerprint_settings.sql new file mode 100644 index 00000000..f1022bb1 --- /dev/null +++ b/internal/db/queries/fingerprint_settings.sql @@ -0,0 +1,15 @@ +-- name: GetFingerprintSettings :one +SELECT * FROM fingerprint_settings WHERE id = true; + +-- name: UpdateFingerprintSettings :one +-- Whole-row write from the admin card; migration 0061's CHECKs are the backstop +-- behind the service's own validation. +UPDATE fingerprint_settings + SET enabled = sqlc.arg(enabled), + chromaprint_length_sec = sqlc.arg(chromaprint_length_sec), + acoustic_max_bit_error_rate = sqlc.arg(acoustic_max_bit_error_rate), + backfill_concurrency = sqlc.arg(backfill_concurrency), + sweep_interval_hours = sqlc.arg(sweep_interval_hours), + updated_at = now() + WHERE id = true +RETURNING *; diff --git a/internal/db/queries/fingerprints.sql b/internal/db/queries/fingerprints.sql index 90a8a978..ae68f233 100644 --- a/internal/db/queries/fingerprints.sql +++ b/internal/db/queries/fingerprints.sql @@ -4,16 +4,17 @@ -- older method. Replaces the row wholesale — a fingerprint of the old bytes has -- no standing once the file has changed. INSERT INTO track_fingerprints ( - track_id, audio_stream_sha256, chromaprint, fingerprint_version + track_id, audio_stream_sha256, chromaprint, fingerprint_version, chromaprint_length_sec ) VALUES ( sqlc.arg(track_id), sqlc.narg(audio_stream_sha256), sqlc.narg(chromaprint), - sqlc.arg(fingerprint_version) + sqlc.arg(fingerprint_version), sqlc.arg(chromaprint_length_sec) ) ON CONFLICT (track_id) DO UPDATE SET - audio_stream_sha256 = EXCLUDED.audio_stream_sha256, - chromaprint = EXCLUDED.chromaprint, - fingerprint_version = EXCLUDED.fingerprint_version, - computed_at = now(); + audio_stream_sha256 = EXCLUDED.audio_stream_sha256, + chromaprint = EXCLUDED.chromaprint, + fingerprint_version = EXCLUDED.fingerprint_version, + chromaprint_length_sec = EXCLUDED.chromaprint_length_sec, + computed_at = now(); -- name: DeleteTrackFingerprint :exec -- A file changed but could not be fingerprinted, for a reason unrelated to the @@ -32,27 +33,37 @@ SELECT t.id, t.file_path FROM tracks t LEFT JOIN track_fingerprints f ON f.track_id = t.id WHERE t.missing_since IS NULL - AND (f.track_id IS NULL OR f.fingerprint_version < sqlc.arg(current_version)) + -- A row taken at another length is as stale as one from an older method: + -- chromaprints at two lengths cannot be compared (#3913). + AND (f.track_id IS NULL + OR f.fingerprint_version < sqlc.arg(current_version) + OR f.chromaprint_length_sec <> sqlc.arg(chromaprint_length_sec)) AND t.id > sqlc.arg(after_id) ORDER BY t.id LIMIT sqlc.arg(batch_limit); -- name: GetFingerprintCoverage :one -- The admin gauge for the backfill. fingerprinted + rejected + pending = total. --- rejected is a row at the current version with a NULL half: a tool ran and --- refused the file, which is settled rather than waiting. Missing tracks are --- excluded, or the gauge could never reach the end. +-- "Current" means derived by the current method AT the current length: a row at +-- another length is pending, because the backfill will re-derive it. rejected is +-- a current row with a NULL half: a tool ran and refused the file, which is +-- settled rather than waiting. Missing tracks are excluded, or the gauge could +-- never reach the end. SELECT count(*)::bigint AS total, count(*) FILTER ( WHERE f.fingerprint_version >= sqlc.arg(current_version) + AND f.chromaprint_length_sec = sqlc.arg(chromaprint_length_sec) AND f.audio_stream_sha256 IS NOT NULL AND f.chromaprint IS NOT NULL )::bigint AS fingerprinted, count(*) FILTER ( WHERE f.fingerprint_version >= sqlc.arg(current_version) + AND f.chromaprint_length_sec = sqlc.arg(chromaprint_length_sec) AND (f.audio_stream_sha256 IS NULL OR f.chromaprint IS NULL) )::bigint AS rejected, count(*) FILTER ( - WHERE f.track_id IS NULL OR f.fingerprint_version < sqlc.arg(current_version) + WHERE f.track_id IS NULL + OR f.fingerprint_version < sqlc.arg(current_version) + OR f.chromaprint_length_sec <> sqlc.arg(chromaprint_length_sec) )::bigint AS pending FROM tracks t LEFT JOIN track_fingerprints f ON f.track_id = t.id diff --git a/internal/db/queries/merge.sql b/internal/db/queries/merge.sql new file mode 100644 index 00000000..6c71dfb1 --- /dev/null +++ b/internal/db/queries/merge.sql @@ -0,0 +1,101 @@ +-- Duplicate merge (Scribe #3911). Every statement here runs inside the one +-- transaction library.MergeDuplicateGroup opens, after the removed copy's file +-- is already gone. The loser's own track row is deleted last with DeleteTrack; +-- what these do is move everything it carries onto the survivor first, so that +-- delete's CASCADE finds nothing left to destroy. + +-- name: LockDuplicateGroupForMerge :one +-- Locks the group for the rest of the transaction, so two merges of one group +-- cannot run at once. +SELECT id, tier, status + FROM duplicate_groups + WHERE id = sqlc.arg(id) + FOR UPDATE; + +-- name: ListDuplicateGroupMergeMembers :many +SELECT t.id, t.file_path, t.file_format, t.file_size, t.added_at, t.album_id, + t.mbid, albums.mbid AS album_mbid + FROM duplicate_group_members m + JOIN tracks t ON t.id = m.track_id + JOIN albums ON albums.id = t.album_id + WHERE m.group_id = sqlc.arg(group_id) + ORDER BY t.id; + +-- Plain repoints: no unique key involves track_id, so moving rows cannot collide. + +-- name: MergeRepointPlayEvents :execrows +UPDATE play_events SET track_id = sqlc.arg(survivor_id)::uuid WHERE track_id = sqlc.arg(loser_id)::uuid; + +-- name: MergeRepointSkipEvents :execrows +UPDATE skip_events SET track_id = sqlc.arg(survivor_id)::uuid WHERE track_id = sqlc.arg(loser_id)::uuid; + +-- name: MergeRepointContextualLikes :execrows +UPDATE contextual_likes SET track_id = sqlc.arg(survivor_id)::uuid WHERE track_id = sqlc.arg(loser_id)::uuid; + +-- name: MergeRepointPlaybackErrors :execrows +UPDATE playback_errors SET track_id = sqlc.arg(survivor_id)::uuid WHERE track_id = sqlc.arg(loser_id)::uuid; + +-- name: MergeRepointLidarrRequests :execrows +UPDATE lidarr_requests SET matched_track_id = sqlc.arg(survivor_id)::uuid + WHERE matched_track_id = sqlc.arg(loser_id)::uuid; + +-- name: MergeRepointPlaylistTracks :many +-- playlist_tracks is keyed by (playlist_id, position), so repointing keeps every +-- entry exactly where it was. A playlist that held both copies simply holds the +-- survivor twice — the user put two entries there, and both stay. +UPDATE playlist_tracks SET track_id = sqlc.arg(survivor_id)::uuid + WHERE track_id = sqlc.arg(loser_id)::uuid +RETURNING playlist_id; + +-- Collision-safe merges: a unique key includes track_id, so the survivor may +-- already hold a matching row. Copy what it lacks; DeleteTrack's CASCADE then +-- removes the loser's originals. + +-- name: MergeCopyGeneralLikes :many +-- One like per user. A user who liked both copies keeps a single like, dated to +-- the earlier of the two. +INSERT INTO general_likes (user_id, track_id, liked_at) +SELECT user_id, sqlc.arg(survivor_id)::uuid, liked_at + FROM general_likes + WHERE track_id = sqlc.arg(loser_id)::uuid +ON CONFLICT (user_id, track_id) DO UPDATE + SET liked_at = LEAST(general_likes.liked_at, EXCLUDED.liked_at) +RETURNING user_id; + +-- name: MergeCopyTrackTags :execrows +INSERT INTO track_tags (track_id, tag, weight) +SELECT sqlc.arg(survivor_id)::uuid, tag, weight + FROM track_tags + WHERE track_id = sqlc.arg(loser_id)::uuid +ON CONFLICT (track_id, tag) DO NOTHING; + +-- name: MergeCopyTrackSimilarity :execrows +-- Rewrites the loser to the survivor on either side of an edge. An edge between +-- the two copies would become a track similar to itself — the table forbids +-- that, and it means nothing — so it is dropped. An edge the survivor already +-- has from the same source is kept as it is. +INSERT INTO track_similarity (track_a_id, track_b_id, score, source, fetched_at) +SELECT CASE WHEN track_a_id = sqlc.arg(loser_id)::uuid THEN sqlc.arg(survivor_id)::uuid ELSE track_a_id END, + CASE WHEN track_b_id = sqlc.arg(loser_id)::uuid THEN sqlc.arg(survivor_id)::uuid ELSE track_b_id END, + score, source, fetched_at + FROM track_similarity + WHERE (track_a_id = sqlc.arg(loser_id)::uuid OR track_b_id = sqlc.arg(loser_id)::uuid) + AND (CASE WHEN track_a_id = sqlc.arg(loser_id)::uuid THEN sqlc.arg(survivor_id)::uuid ELSE track_a_id END) + <> (CASE WHEN track_b_id = sqlc.arg(loser_id)::uuid THEN sqlc.arg(survivor_id)::uuid ELSE track_b_id END) +ON CONFLICT (track_a_id, track_b_id, source) DO NOTHING; + +-- name: MergeInheritTrackMbid :exec +-- A recording MBID is what the similarity pipeline keys on. If only the removed +-- copy carried one, the survivor takes it rather than going dark to similarity. +UPDATE tracks AS survivor + SET mbid = loser.mbid + FROM tracks AS loser + WHERE survivor.id = sqlc.arg(survivor_id)::uuid + AND loser.id = sqlc.arg(loser_id)::uuid + AND survivor.mbid IS NULL + AND loser.mbid IS NOT NULL; + +-- name: MarkDuplicateGroupMerged :execrows +UPDATE duplicate_groups + SET status = 'merged', resolved_at = now() + WHERE id = sqlc.arg(id) AND status = 'pending'; diff --git a/internal/db/queries/tracks.sql b/internal/db/queries/tracks.sql index 1f4719a2..e8075668 100644 --- a/internal/db/queries/tracks.sql +++ b/internal/db/queries/tracks.sql @@ -155,17 +155,19 @@ SELECT id, file_path FROM tracks AND mbid = sqlc.arg(mbid)::text LIMIT 2; --- name: FindMissingTrackByFingerprint :many --- Move detection fallback for files with no MBID (#2528). Exact byte size AND --- exact decoded duration is a strong pair: a plain move or rename preserves --- both, while a re-encode changes at least one — and a re-encode genuinely is a --- different file, so failing to match there is correct rather than a gap. +-- name: FindMissingTrackByAudioHash :many +-- Move detection fallback for files with no MBID (#2528, #3914). The audio stream +-- hash identifies the encoded audio itself, so it survives a rename, a move and a +-- retag — anything short of a re-encode. It replaced (file_size, duration_ms), +-- which could pair two unrelated files that happened to share a byte count and a +-- duration, and missed a file retagged in place, whose size changes. -- -- Same missing-only constraint and same LIMIT 2 rationale as the MBID variant. -SELECT id, file_path FROM tracks - WHERE missing_since IS NOT NULL - AND file_size = sqlc.arg(file_size) - AND duration_ms = sqlc.arg(duration_ms) +SELECT t.id, t.file_path + FROM tracks t + JOIN track_fingerprints f ON f.track_id = t.id + WHERE t.missing_since IS NOT NULL + AND f.audio_stream_sha256 = sqlc.arg(audio_stream_sha256) LIMIT 2; -- name: AdoptTrackPath :execrows diff --git a/internal/dbtest/reset.go b/internal/dbtest/reset.go index a99db739..a5b2380e 100644 --- a/internal/dbtest/reset.go +++ b/internal/dbtest/reset.go @@ -87,6 +87,9 @@ var dataTables = []string{ // pristine Discover knobs rather than whatever a previous test tuned. "discover_tuning", "recommendation_tuning_audit", + "duplicate_group_members", // M400 + "duplicate_groups", + "duplicate_sweeps", "track_fingerprints", // M400 "tracks", "albums", @@ -127,4 +130,15 @@ func ResetDB(t *testing.T, pool *pgxpool.Pool) { ); err != nil { t.Fatalf("dbtest.ResetDB reset tag-sources version: %v", err) } + // Fingerprinting settings (M400 #3913), a singleton like the counters above. + // Every column goes back to its migration default rather than to literals + // written here, so a test can pin the Go defaults to the migration's. + if _, err := pool.Exec(ctx, ` + UPDATE fingerprint_settings + SET enabled = DEFAULT, chromaprint_length_sec = DEFAULT, + acoustic_max_bit_error_rate = DEFAULT, backfill_concurrency = DEFAULT, + sweep_interval_hours = DEFAULT, updated_at = DEFAULT`, + ); err != nil { + t.Fatalf("dbtest.ResetDB reset fingerprint settings: %v", err) + } } diff --git a/internal/library/delete.go b/internal/library/delete.go index 70d3ece5..a12aec3c 100644 --- a/internal/library/delete.go +++ b/internal/library/delete.go @@ -114,10 +114,8 @@ func DeleteTrackFile( return DeletedTrack{}, fmt.Errorf("get track: %w", err) } - if err := removeFile(track.FilePath); err != nil && !errors.Is(err, fs.ErrNotExist) { - return DeletedTrack{}, &FileRemoveError{ - Path: track.FilePath, UID: os.Getuid(), GID: os.Getgid(), Err: err, - } + if err := removeTrackFileOnDisk(track.FilePath); err != nil { + return DeletedTrack{}, err } // The row and any album or artist it empties go together, so a failure @@ -138,25 +136,9 @@ func DeleteTrackFile( return DeletedTrack{}, fmt.Errorf("delete track: %w", err) } - var out DeletedTrack - album, err := tq.DeleteAlbumIfEmpty(ctx, deleted.AlbumID) - switch { - case err == nil: - albumID := album.ID - out.AlbumID = &albumID - artistID, aerr := tq.DeleteArtistIfEmpty(ctx, album.ArtistID) - switch { - case aerr == nil: - out.ArtistID = &artistID - case errors.Is(aerr, pgx.ErrNoRows): - // The artist still has other albums or stray tracks. - default: - return DeletedTrack{}, fmt.Errorf("delete artist if empty: %w", aerr) - } - case errors.Is(err, pgx.ErrNoRows): - // The album still has other tracks. - default: - return DeletedTrack{}, fmt.Errorf("delete album if empty: %w", err) + out, err := tidyEmptiedAlbum(ctx, tq, deleted.AlbumID) + if err != nil { + return DeletedTrack{}, err } if err := tx.Commit(ctx); err != nil { @@ -179,3 +161,41 @@ func DeleteTrackFile( } return out, nil } + +// removeTrackFileOnDisk is the one rule for removing a track's file, shared by +// DeleteTrackFile and the duplicate merge. A file already gone is fine; anything +// else comes back as a *FileRemoveError, and the caller must then change nothing +// in the database (#3918). +func removeTrackFileOnDisk(path string) error { + if err := removeFile(path); err != nil && !errors.Is(err, fs.ErrNotExist) { + return &FileRemoveError{Path: path, UID: os.Getuid(), GID: os.Getgid(), Err: err} + } + return nil +} + +// tidyEmptiedAlbum removes an album a track delete left with no tracks, and its +// artist if that album was the artist's last. It runs on the caller's +// transaction, so the tidy-up commits or rolls back with the delete itself. +func tidyEmptiedAlbum(ctx context.Context, tq *dbq.Queries, albumID pgtype.UUID) (DeletedTrack, error) { + var out DeletedTrack + album, err := tq.DeleteAlbumIfEmpty(ctx, albumID) + switch { + case err == nil: + id := album.ID + out.AlbumID = &id + artistID, aerr := tq.DeleteArtistIfEmpty(ctx, album.ArtistID) + switch { + case aerr == nil: + out.ArtistID = &artistID + case errors.Is(aerr, pgx.ErrNoRows): + // The artist still has other albums or stray tracks. + default: + return DeletedTrack{}, fmt.Errorf("delete artist if empty: %w", aerr) + } + case errors.Is(err, pgx.ErrNoRows): + // The album still has other tracks. + default: + return DeletedTrack{}, fmt.Errorf("delete album if empty: %w", err) + } + return out, nil +} diff --git a/internal/library/duplicate_match.go b/internal/library/duplicate_match.go new file mode 100644 index 00000000..d45b83f4 --- /dev/null +++ b/internal/library/duplicate_match.go @@ -0,0 +1,430 @@ +package library + +import ( + "math" + "math/bits" + "sort" +) + +// Duplicate matching (M400 #3909). +// +// Pure functions over fingerprints: no database, no files. This is the part that +// decides whether two tracks in the operator's library are proposed as one +// recording, so every rule in it has to be falsifiable in a unit test. +// +// Two tiers, answering different questions: +// +// exact equal audio_stream_sha256 — the same encoded audio bytes. No score, +// no threshold, no false positives (the #3885 pair). +// acoustic chromaprint fingerprints that agree closely once aligned — the +// same recording at another bitrate or in another codec. +// +// The acoustic comparison follows the approach of AcoustID's pg_acoustid +// (acoustid_compare.c): vote on the relative offset between two fingerprints +// using items that agree in their high bits, then measure disagreement at the +// winning offset. Reimplemented from that description; no code was copied. +// The alignment window and match-bit width below are taken from it. + +// maxAlignOffsetItems bounds how far apart two fingerprints may be shifted and +// still be compared: ±120 items, about 15 seconds at chromaprint's ~8 items per +// second. Covers a leading silence trimmed differently or a short intro; the +// same bound pg_acoustid uses (ACOUSTID_MAX_ALIGN_OFFSET). +const maxAlignOffsetItems = 120 + +// alignMatchBits is how many high bits two items must share to vote for an +// offset. Matching whole 32-bit items would miss the same recording at another +// bitrate, whose low bits are noisier; 14 is pg_acoustid's MATCH_BITS. +const alignMatchBits = 14 + +// minOverlapItems is the least overlap worth a verdict: about 10 seconds. A few +// items agreeing perfectly is not evidence that two recordings are one. +const minOverlapItems = 80 + +// minDistinctFraction rejects low-information fingerprints before they can +// match. Near-silence, a sustained tone or a click track produces the same few +// items over and over, and two such tracks agree closely without being the +// same recording. Real music is overwhelmingly distinct item to item, so this +// floor only catches the pathological case. A judgment value, not a measured +// one — revisit if the sweep reports real tracks refused for it. +const minDistinctFraction = 0.3 + +// defaultAcousticMaxBitErrorRate is the most disagreement two aligned +// fingerprints may show and still be proposed as one recording. Unrelated audio +// sits near 0.5; the same recording re-encoded lands well under 0.1. +// +// Deliberately conservative. The operator's stated worry is the opposite of a +// missed duplicate: "the same song can appear in different albums, usually it's +// a different recording", and an instrumental shares its vocal version's +// harmony, which chroma features capture. A false merge is the failure that +// matters, and the report is reviewed anyway. This is an unmeasured default: +// calibrate it against real pairs once the backfill (#3908) has populated the +// library, then expose it in Settings (#3913). +const defaultAcousticMaxBitErrorRate = 0.15 + +// durationToleranceMs is how far apart two tracks' durations may be and still be +// compared. Encoders pad and trim a little; different edits differ by more. +const durationToleranceMs = 3000 + +// maxAcousticGroupSize caps an acoustic group. A cluster bigger than this is far +// more likely a shared jingle, a skit or a low-information pattern than eight +// copies of one recording, and proposing it would bury the real duplicates. +// Exact-tier groups are not capped: identical bytes are identical however many. +const maxAcousticGroupSize = 8 + +// acousticScore is the result of comparing two fingerprints. +type acousticScore struct { + // Offset is how many items b is shifted against a: b[i+Offset] aligns with + // a[i]. + Offset int + // Overlap is how many aligned items were compared. + Overlap int + // BitErrorRate is the fraction of differing bits over the overlap, 0..1. + BitErrorRate float64 +} + +// preparedPrint is a fingerprint with the parts every comparison needs worked +// out once. The sweep compares each track with every other track within a few +// seconds of its duration, so rebuilding the alignment index for each pair would +// dominate its cost. +type preparedPrint struct { + items []int32 + index map[uint32][]int + informative bool +} + +// preparePrint indexes a fingerprint's items by their high bits and records +// whether it varies enough to be compared at all. +func preparePrint(fp []int32) *preparedPrint { + // Each bucket keeps only a few positions: a value repeating many times is + // uninformative, and letting it vote once per repeat would make every + // pairing O(n²). + const keepPerBucket = 4 + p := &preparedPrint{items: fp, index: make(map[uint32][]int, len(fp))} + seen := make(map[int32]struct{}, len(fp)) + for i, v := range fp { + seen[v] = struct{}{} + key := alignKey(v) + if pos := p.index[key]; len(pos) < keepPerBucket { + p.index[key] = append(pos, i) + } + } + p.informative = len(fp) > 0 && float64(len(seen)) >= minDistinctFraction*float64(len(fp)) + return p +} + +func alignKey(v int32) uint32 { return uint32(v) >> (32 - alignMatchBits) } + +// compareChromaprint aligns two raw fingerprints and measures how much they +// disagree. ok is false when no verdict is possible: no offset gathered any +// votes, the overlap at the best offset is too short, or either side carries +// too little information to mean anything. +func compareChromaprint(a, b []int32) (acousticScore, bool) { + return comparePrepared(preparePrint(a), preparePrint(b)) +} + +// comparePrepared is compareChromaprint over fingerprints already prepared. +func comparePrepared(a, b *preparedPrint) (acousticScore, bool) { + if len(a.items) < minOverlapItems || len(b.items) < minOverlapItems { + return acousticScore{}, false + } + if !a.informative || !b.informative { + return acousticScore{}, false + } + + offset, ok := bestOffset(a, b) + if !ok { + return acousticScore{}, false + } + + // a[i] aligns with b[i+offset]; walk the indices valid on both sides. + start := max(0, -offset) + end := min(len(a.items), len(b.items)-offset) + overlap := end - start + if overlap < minOverlapItems { + return acousticScore{}, false + } + errBits := 0 + for i := start; i < end; i++ { + errBits += bits.OnesCount32(uint32(a.items[i]) ^ uint32(b.items[i+offset])) + } + return acousticScore{ + Offset: offset, + Overlap: overlap, + BitErrorRate: float64(errBits) / float64(32*overlap), + }, true +} + +// bestOffset returns the relative shift most items agree on. +func bestOffset(a, b *preparedPrint) (int, bool) { + votes := make([]int, 2*maxAlignOffsetItems+1) + for j, v := range b.items { + for _, i := range a.index[alignKey(v)] { + off := j - i + if off >= -maxAlignOffsetItems && off <= maxAlignOffsetItems { + votes[off+maxAlignOffsetItems]++ + } + } + } + + best, bestVotes := 0, 0 + for k, n := range votes { + // Strictly greater keeps the smallest shift on a tie, which is the more + // likely truth and keeps the result deterministic. + if n > bestVotes || (n == bestVotes && n > 0 && abs(k-maxAlignOffsetItems) < abs(best)) { + best, bestVotes = k-maxAlignOffsetItems, n + } + } + return best, bestVotes > 0 +} + +// fingerprintCandidate is one track as the grouping sees it. +type fingerprintCandidate struct { + ID string + DurationMs int32 + StreamSHA256 []byte + Chromaprint []int32 +} + +// duplicateTier names what a group's evidence is. +type duplicateTier string + +const ( + tierExact duplicateTier = "exact" + tierAcoustic duplicateTier = "acoustic" +) + +// duplicateGroup is a set of tracks proposed as one recording. Members are +// sorted by ID. +type duplicateGroup struct { + Tier duplicateTier + Members []string + // WorstBitErrorRate is the largest disagreement between any two members of + // an acoustic group — the weakest evidence the group rests on. Zero for + // exact groups. + WorstBitErrorRate float64 +} + +// groupingResult is what one grouping pass found. +type groupingResult struct { + Groups []duplicateGroup + // OversizeClusters counts acoustic clusters discarded for exceeding + // maxAcousticGroupSize. Reported rather than silent: a sudden rise means the + // cap or the information floor needs attention. + OversizeClusters int +} + +// groupUnit is one thing the acoustic pass compares: a single track, or an exact +// group standing in for all its byte-identical copies. +type groupUnit struct { + ids []string // every member, sorted + durationMs int32 + sortKey string // the representative's id: ties on duration break on it + print *preparedPrint + exact bool // more than one member with identical audio + assigned bool +} + +// streamGrouper is the acoustic pass over units arriving in (durationMs, sortKey) +// order. It holds only the units within durationToleranceMs of the oldest one +// not yet settled, so memory is bounded by the densest few seconds of the +// library rather than by its size — the whole library's fingerprints would be +// hundreds of megabytes. +// +// A seed can be settled as soon as a unit arrives beyond its window: everything +// it could group with has already arrived, and no later seed can reach back to +// it because seeds only look forward. That is what makes the streamed result +// identical to running the same pass over the whole sorted list. +// +// Grouping is COMPLETE-LINKAGE: a unit joins a group only if it matches every +// unit already in it, within the duration tolerance and the bit-error limit. +// Single-linkage would let a chain of near-misses — A close to B, B close to C — +// drag A and C, which are not close, into one proposed merge. Complete linkage +// also means any member can be chosen as the survivor (#3911). +// +// When an acoustic group absorbs an exact group, the result is tier acoustic: a +// group is only as certain as its weakest link. +type streamGrouper struct { + maxBitErrorRate float64 + window []*groupUnit + res groupingResult +} + +func newStreamGrouper(maxBitErrorRate float64) *streamGrouper { + return &streamGrouper{maxBitErrorRate: maxBitErrorRate} +} + +// push adds the next unit. Units must arrive in non-decreasing +// (durationMs, sortKey) order. +func (g *streamGrouper) push(u *groupUnit) { + g.window = append(g.window, u) + for len(g.window) > 1 && u.durationMs-g.window[0].durationMs > durationToleranceMs { + g.settleOldest() + } +} + +// finish settles every unit still waiting and returns what was found. Groups +// are in no particular order; callers sort with sortGroups. +func (g *streamGrouper) finish() groupingResult { + for len(g.window) > 0 { + g.settleOldest() + } + return g.res +} + +func (g *streamGrouper) settleOldest() { + seed := g.window[0] + g.window[0] = nil // release it: the window's backing array outlives the slide + g.window = g.window[1:] + if seed.assigned { + return + } + + group := []*groupUnit{seed} + worst := 0.0 + for _, cand := range g.window { + if cand.durationMs-seed.durationMs > durationToleranceMs { + break + } + if cand.assigned { + continue + } + joined, worstWithCand := true, worst + for _, member := range group { + if abs32(cand.durationMs-member.durationMs) > durationToleranceMs { + joined = false + break + } + score, ok := comparePrepared(member.print, cand.print) + if !ok || score.BitErrorRate > g.maxBitErrorRate { + joined = false + break + } + worstWithCand = math.Max(worstWithCand, score.BitErrorRate) + } + if joined { + group = append(group, cand) + worst = worstWithCand + } + } + + if len(group) == 1 { + if seed.exact { + g.res.Groups = append(g.res.Groups, duplicateGroup{Tier: tierExact, Members: seed.ids}) + } + return + } + // Count units, not tracks: an absorbed exact group is one piece of acoustic + // evidence however many identical files it holds. + if len(group) > maxAcousticGroupSize { + g.res.OversizeClusters++ + for _, member := range group { + member.assigned = true + // The acoustic evidence is untrustworthy; identical bytes are not. + // An exact group caught inside an oversize cluster is still proposed. + if member.exact { + g.res.Groups = append(g.res.Groups, duplicateGroup{Tier: tierExact, Members: member.ids}) + } + } + return + } + var members []string + for _, member := range group { + member.assigned = true + members = append(members, member.ids...) + } + sort.Strings(members) + g.res.Groups = append(g.res.Groups, duplicateGroup{ + Tier: tierAcoustic, Members: members, WorstBitErrorRate: worst, + }) +} + +// groupDuplicates proposes duplicate groups among candidates held in memory. It +// runs the same streamGrouper the sweep uses, so there is one grouping rule. +// +// Exact groups come first: tracks sharing an audio stream hash. Each becomes a +// single unit for the acoustic pass, represented by its member with the lowest +// (duration, id) that has a chromaprint. That is the member the sweep's +// duration-ordered stream meets first, which keeps the two identical. An exact +// group with no chromaprint at all cannot be compared acoustically and stands +// on its own. +// +// The output does not depend on input order. +func groupDuplicates(cands []fingerprintCandidate, maxBitErrorRate float64) groupingResult { + byHash := map[string][]fingerprintCandidate{} + var units []*groupUnit + var printless []duplicateGroup + for _, c := range cands { + if len(c.StreamSHA256) > 0 { + byHash[string(c.StreamSHA256)] = append(byHash[string(c.StreamSHA256)], c) + continue + } + if len(c.Chromaprint) > 0 { + units = append(units, &groupUnit{ + ids: []string{c.ID}, durationMs: c.DurationMs, sortKey: c.ID, print: preparePrint(c.Chromaprint), + }) + } + } + for _, group := range byHash { + ids := make([]string, len(group)) + for i, m := range group { + ids[i] = m.ID + } + sort.Strings(ids) + + var rep *fingerprintCandidate + for i := range group { + m := &group[i] + if len(m.Chromaprint) == 0 { + continue + } + if rep == nil || m.DurationMs < rep.DurationMs || (m.DurationMs == rep.DurationMs && m.ID < rep.ID) { + rep = m + } + } + if rep == nil { + if len(group) > 1 { + printless = append(printless, duplicateGroup{Tier: tierExact, Members: ids}) + } + continue + } + units = append(units, &groupUnit{ + ids: ids, durationMs: rep.DurationMs, sortKey: rep.ID, + print: preparePrint(rep.Chromaprint), exact: len(group) > 1, + }) + } + + sort.Slice(units, func(i, j int) bool { + if units[i].durationMs != units[j].durationMs { + return units[i].durationMs < units[j].durationMs + } + return units[i].sortKey < units[j].sortKey + }) + g := newStreamGrouper(maxBitErrorRate) + for _, u := range units { + g.push(u) + } + res := g.finish() + res.Groups = append(res.Groups, printless...) + sortGroups(res.Groups) + return res +} + +// sortGroups orders groups by their first member. Groups are disjoint, so that +// is a total order. +func sortGroups(groups []duplicateGroup) { + sort.Slice(groups, func(i, j int) bool { return groups[i].Members[0] < groups[j].Members[0] }) +} + +func abs(n int) int { + if n < 0 { + return -n + } + return n +} + +func abs32(n int32) int32 { + if n < 0 { + return -n + } + return n +} diff --git a/internal/library/duplicate_match_test.go b/internal/library/duplicate_match_test.go new file mode 100644 index 00000000..f0d2decf --- /dev/null +++ b/internal/library/duplicate_match_test.go @@ -0,0 +1,241 @@ +package library + +import ( + "math" + "math/rand/v2" + "reflect" + "testing" +) + +// printLen is a realistic fingerprint length: fpcalc's 120s at ~8 items/second. +const printLen = 960 + +// randomPrint is a deterministic stand-in for one recording's fingerprint. +func randomPrint(seed uint64, n int) []int32 { + r := rand.New(rand.NewPCG(seed, seed^0x9e3779b97f4a7c15)) + fp := make([]int32, n) + for i := range fp { + fp[i] = int32(r.Uint32()) + } + return fp +} + +// withBitNoise flips exactly round(fraction × all bits) distinct bits — the +// same recording through a different encoder, at a known bit-error rate. +func withBitNoise(fp []int32, fraction float64, seed uint64) []int32 { + out := append([]int32(nil), fp...) + r := rand.New(rand.NewPCG(seed, seed^0x243f6a8885a308d3)) + total := 32 * len(fp) + for _, pos := range r.Perm(total)[:int(math.Round(fraction*float64(total)))] { + out[pos/32] ^= int32(uint32(1) << (pos % 32)) + } + return out +} + +func constantPrint(v int32, n int) []int32 { + fp := make([]int32, n) + for i := range fp { + fp[i] = v + } + return fp +} + +func TestCompareChromaprint(t *testing.T) { + base := randomPrint(1, printLen) + + t.Run("identical", func(t *testing.T) { + got, ok := compareChromaprint(base, base) + if !ok || got.BitErrorRate != 0 || got.Offset != 0 || got.Overlap != printLen { + t.Fatalf("got %+v ok=%v, want an exact alignment", got, ok) + } + }) + + t.Run("re-encoded: known bit noise is measured exactly", func(t *testing.T) { + got, ok := compareChromaprint(base, withBitNoise(base, 0.03, 2)) + if !ok { + t.Fatal("a re-encode was not comparable") + } + if want := math.Round(0.03*32*printLen) / (32 * printLen); got.BitErrorRate != want { + t.Fatalf("BitErrorRate = %v, want %v", got.BitErrorRate, want) + } + }) + + // b starts 40 items later in the same audio: b[j] = a[j+40], so a[i] aligns + // with b[i-40]. + t.Run("offset inside the window is recovered", func(t *testing.T) { + got, ok := compareChromaprint(base, base[40:]) + if !ok || got.Offset != -40 || got.BitErrorRate != 0 || got.Overlap != printLen-40 { + t.Fatalf("got %+v ok=%v, want offset -40 with no error", got, ok) + } + }) + + t.Run("offset beyond the window never matches", func(t *testing.T) { + got, ok := compareChromaprint(base, base[200:]) + if ok && got.BitErrorRate <= defaultAcousticMaxBitErrorRate { + t.Fatalf("a 200-item shift matched: %+v", got) + } + }) + + t.Run("unrelated recordings sit near 0.5", func(t *testing.T) { + got, ok := compareChromaprint(base, randomPrint(99, printLen)) + if ok && got.BitErrorRate < 0.4 { + t.Fatalf("unrelated fingerprints scored %v", got.BitErrorRate) + } + }) + + t.Run("too short an overlap gives no verdict", func(t *testing.T) { + if got, ok := compareChromaprint(base, base[:minOverlapItems-1]); ok { + t.Fatalf("a %d-item fingerprint was compared: %+v", minOverlapItems-1, got) + } + }) + + // Two near-silent tracks agree perfectly without being one recording. The + // information floor is the only thing standing between them and a merge. + t.Run("low-information fingerprints give no verdict", func(t *testing.T) { + silence := constantPrint(0x1234, printLen) + if got, ok := compareChromaprint(silence, silence); ok { + t.Fatalf("silence compared as a match: %+v", got) + } + }) + + t.Run("the threshold separates close from not close", func(t *testing.T) { + near, _ := compareChromaprint(base, withBitNoise(base, 0.10, 3)) + far, _ := compareChromaprint(base, withBitNoise(base, 0.20, 4)) + if near.BitErrorRate > defaultAcousticMaxBitErrorRate { + t.Errorf("10%% noise (%v) is over the threshold", near.BitErrorRate) + } + if far.BitErrorRate <= defaultAcousticMaxBitErrorRate { + t.Errorf("20%% noise (%v) is under the threshold", far.BitErrorRate) + } + }) +} + +func TestGroupDuplicates_ExactTier(t *testing.T) { + hash := []byte("sha256-of-www-instrumental-bytes") + res := groupDuplicates([]fingerprintCandidate{ + {ID: "www-01", DurationMs: 215000, StreamSHA256: hash}, + {ID: "www-02", DurationMs: 215000, StreamSHA256: hash}, + {ID: "lovesick", DurationMs: 198000, StreamSHA256: []byte("another")}, + }, defaultAcousticMaxBitErrorRate) + want := []duplicateGroup{{Tier: tierExact, Members: []string{"www-01", "www-02"}}} + if !reflect.DeepEqual(res.Groups, want) { + t.Fatalf("groups = %+v, want %+v", res.Groups, want) + } +} + +func TestGroupDuplicates_AcousticPair(t *testing.T) { + p := randomPrint(10, printLen) + res := groupDuplicates([]fingerprintCandidate{ + {ID: "album", DurationMs: 240000, Chromaprint: p}, + {ID: "compilation", DurationMs: 241000, Chromaprint: withBitNoise(p, 0.05, 11)}, + }, defaultAcousticMaxBitErrorRate) + if len(res.Groups) != 1 || res.Groups[0].Tier != tierAcoustic || + !reflect.DeepEqual(res.Groups[0].Members, []string{"album", "compilation"}) { + t.Fatalf("groups = %+v, want one acoustic pair", res.Groups) + } + if got := res.Groups[0].WorstBitErrorRate; math.Abs(got-0.05) > 0.001 { + t.Fatalf("WorstBitErrorRate = %v, want about 0.05", got) + } +} + +// A is close to B and B is close to C, but A and C are not close. Under +// single linkage all three would be proposed as one recording; complete linkage +// must keep C out. +func TestGroupDuplicates_NoChaining(t *testing.T) { + a := randomPrint(20, printLen) + b := withBitNoise(a, 0.10, 21) + c := withBitNoise(b, 0.10, 22) + if s, _ := compareChromaprint(a, c); s.BitErrorRate <= defaultAcousticMaxBitErrorRate { + t.Fatalf("fixture broken: A and C are close (%v), so this cannot test chaining", s.BitErrorRate) + } + res := groupDuplicates([]fingerprintCandidate{ + {ID: "a", DurationMs: 200000, Chromaprint: a}, + {ID: "b", DurationMs: 200000, Chromaprint: b}, + {ID: "c", DurationMs: 200000, Chromaprint: c}, + }, defaultAcousticMaxBitErrorRate) + if len(res.Groups) != 1 || !reflect.DeepEqual(res.Groups[0].Members, []string{"a", "b"}) { + t.Fatalf("groups = %+v, want only {a, b}", res.Groups) + } +} + +func TestGroupDuplicates_DurationTolerance(t *testing.T) { + p := randomPrint(30, printLen) + res := groupDuplicates([]fingerprintCandidate{ + {ID: "edit", DurationMs: 200000, Chromaprint: p}, + {ID: "extended", DurationMs: 200000 + durationToleranceMs + 1, Chromaprint: p}, + }, defaultAcousticMaxBitErrorRate) + if len(res.Groups) != 0 { + t.Fatalf("tracks %dms apart were grouped: %+v", durationToleranceMs+1, res.Groups) + } +} + +// Nine tracks that all match are far likelier a shared jingle than nine copies +// of one recording. The cluster must be reported, not proposed. +func TestGroupDuplicates_OversizeClusterIsDiscarded(t *testing.T) { + p := randomPrint(40, printLen) + var cands []fingerprintCandidate + for i := range maxAcousticGroupSize + 1 { + cands = append(cands, fingerprintCandidate{ + ID: string(rune('a' + i)), DurationMs: 30000, Chromaprint: withBitNoise(p, 0.01, uint64(100+i)), + }) + } + res := groupDuplicates(cands, defaultAcousticMaxBitErrorRate) + if len(res.Groups) != 0 || res.OversizeClusters != 1 { + t.Fatalf("groups = %+v, oversize = %d; want none proposed and 1 oversize", res.Groups, res.OversizeClusters) + } +} + +// Two byte-identical copies plus a re-encode of the same recording are one +// group, and it is only as certain as its weakest link. +func TestGroupDuplicates_ExactGroupAbsorbedIntoAcoustic(t *testing.T) { + p := randomPrint(50, printLen) + hash := []byte("same-bytes") + res := groupDuplicates([]fingerprintCandidate{ + {ID: "x1", DurationMs: 180000, StreamSHA256: hash, Chromaprint: p}, + {ID: "x2", DurationMs: 180000, StreamSHA256: hash, Chromaprint: p}, + {ID: "y", DurationMs: 180000, StreamSHA256: []byte("other-bytes"), Chromaprint: withBitNoise(p, 0.03, 51)}, + }, defaultAcousticMaxBitErrorRate) + want := []string{"x1", "x2", "y"} + if len(res.Groups) != 1 || res.Groups[0].Tier != tierAcoustic || !reflect.DeepEqual(res.Groups[0].Members, want) { + t.Fatalf("groups = %+v, want one acoustic group %v", res.Groups, want) + } +} + +func TestGroupDuplicates_UnrelatedTracksNeverGroup(t *testing.T) { + var cands []fingerprintCandidate + for i := range 6 { + cands = append(cands, fingerprintCandidate{ + ID: string(rune('a' + i)), DurationMs: 210000, Chromaprint: randomPrint(uint64(60+i), printLen), + }) + } + if res := groupDuplicates(cands, defaultAcousticMaxBitErrorRate); len(res.Groups) != 0 { + t.Fatalf("unrelated recordings were grouped: %+v", res.Groups) + } +} + +func TestGroupDuplicates_OrderIndependent(t *testing.T) { + p := randomPrint(70, printLen) + q := randomPrint(71, printLen) + hash := []byte("identical") + cands := []fingerprintCandidate{ + {ID: "p1", DurationMs: 200000, Chromaprint: p}, + {ID: "p2", DurationMs: 201000, Chromaprint: withBitNoise(p, 0.04, 72)}, + {ID: "q1", DurationMs: 150000, Chromaprint: q}, + {ID: "q2", DurationMs: 150500, Chromaprint: withBitNoise(q, 0.02, 73)}, + {ID: "h1", DurationMs: 90000, StreamSHA256: hash}, + {ID: "h2", DurationMs: 90000, StreamSHA256: hash}, + {ID: "lone", DurationMs: 200000, Chromaprint: randomPrint(74, printLen)}, + } + want := groupDuplicates(cands, defaultAcousticMaxBitErrorRate) + if len(want.Groups) != 3 { + t.Fatalf("fixture broken: %d groups, want 3 (p, q, h)", len(want.Groups)) + } + r := rand.New(rand.NewPCG(75, 76)) + for range 20 { + shuffled := append([]fingerprintCandidate(nil), cands...) + r.Shuffle(len(shuffled), func(i, j int) { shuffled[i], shuffled[j] = shuffled[j], shuffled[i] }) + if got := groupDuplicates(shuffled, defaultAcousticMaxBitErrorRate); !reflect.DeepEqual(got, want) { + t.Fatalf("input order changed the result:\n got %+v\n want %+v", got, want) + } + } +} diff --git a/internal/library/duplicate_merge.go b/internal/library/duplicate_merge.go new file mode 100644 index 00000000..1182c510 --- /dev/null +++ b/internal/library/duplicate_merge.go @@ -0,0 +1,309 @@ +package library + +import ( + "context" + "errors" + "fmt" + "log/slog" + "sort" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/coverart" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" +) + +// Duplicate merge (M400 #3911). + +// ErrDuplicateGroupNotPending means the group was already merged or dismissed, +// no longer exists, or no longer has two members to merge. +var ErrDuplicateGroupNotPending = errors.New("library: duplicate group is not pending") + +// ErrSurvivorNotInGroup means the copy chosen to keep is not a member of the group. +var ErrSurvivorNotInGroup = errors.New("library: survivor is not a member of the group") + +// MergedCopy is one copy a merge kept or removed. +type MergedCopy struct { + TrackID pgtype.UUID + FilePath string + TrackMbid *string + AlbumMbid *string +} + +// MergeResult says what a merge did. +type MergeResult struct { + Tier string + Survivor MergedCopy + Removed []MergedCopy + + // What moved onto the survivor — reported so the operator, and the audit + // log, can see that the history was kept rather than take it on trust. + PlayEvents int64 + SkipEvents int64 + Likes int // users whose like now sits on the survivor + PlaylistEntries int + + DeletedAlbumIDs []pgtype.UUID + DeletedArtistIDs []pgtype.UUID +} + +// MergeDuplicateGroup keeps one copy of a duplicate group and removes the rest, +// carrying everything the removed copies held onto the one kept. +// +// survivorID chooses the copy to keep; an invalid (zero) id takes the proposal +// from ProposeSurvivor. +// +// The danger this is built around: every table referencing tracks does so ON +// DELETE CASCADE, so deleting a duplicate's row outright silently destroys its +// likes, plays, playlist entries and tags. The merge moves all of that onto the +// survivor first, and only then deletes the now-empty row. +// +// It deletes the removed copies' FILES too, and first, before any row changes +// (#3918, note #3926). A merge that left the file behind would be undone by the +// next scan, which re-imports it as a new track with no history. If a file cannot +// be removed, the *FileRemoveError comes back and nothing in the database changes. +// With several copies to remove, one file may already be gone when a later one +// fails; that copy's row keeps all its history and is marked missing by the next +// scan, and retrying the merge picks up where it stopped. +// +// Everything else happens in one transaction, which holds a lock on the group so +// two merges of it cannot run at once. Sync changes for clients' caches are logged +// inside it, the way the playlists service logs its own. +func MergeDuplicateGroup( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, + groupID, survivorID pgtype.UUID, +) (MergeResult, error) { + if logger == nil { + logger = slog.Default() + } + tx, err := pool.Begin(ctx) + if err != nil { + return MergeResult{}, fmt.Errorf("begin merge: %w", err) + } + defer func() { _ = tx.Rollback(ctx) }() + tq := dbq.New(tx) + + group, err := tq.LockDuplicateGroupForMerge(ctx, groupID) + if errors.Is(err, pgx.ErrNoRows) { + return MergeResult{}, ErrDuplicateGroupNotPending + } + if err != nil { + return MergeResult{}, fmt.Errorf("lock duplicate group: %w", err) + } + if group.Status != "pending" { + return MergeResult{}, ErrDuplicateGroupNotPending + } + + members, err := tq.ListDuplicateGroupMergeMembers(ctx, groupID) + if err != nil { + return MergeResult{}, fmt.Errorf("list group members: %w", err) + } + if len(members) < 2 { + return MergeResult{}, ErrDuplicateGroupNotPending + } + survivor, losers, err := splitSurvivor(members, survivorID) + if err != nil { + return MergeResult{}, err + } + + for _, l := range losers { + if err := removeTrackFileOnDisk(l.FilePath); err != nil { + return MergeResult{}, err + } + } + + res := MergeResult{Tier: group.Tier, Survivor: mergedCopyOf(survivor)} + likers := map[string]struct{}{} + changes := mergeChanges{} + survivorKey := syncpkg.FormatUUID(survivor.ID) + + for _, l := range losers { + ids := struct{ survivor, loser pgtype.UUID }{survivor.ID, l.ID} + loserKey := syncpkg.FormatUUID(l.ID) + + n, err := tq.MergeRepointPlayEvents(ctx, dbq.MergeRepointPlayEventsParams{SurvivorID: ids.survivor, LoserID: ids.loser}) + if err != nil { + return MergeResult{}, fmt.Errorf("move play events: %w", err) + } + res.PlayEvents += n + n, err = tq.MergeRepointSkipEvents(ctx, dbq.MergeRepointSkipEventsParams{SurvivorID: ids.survivor, LoserID: ids.loser}) + if err != nil { + return MergeResult{}, fmt.Errorf("move skip events: %w", err) + } + res.SkipEvents += n + if _, err := tq.MergeRepointContextualLikes(ctx, dbq.MergeRepointContextualLikesParams{SurvivorID: ids.survivor, LoserID: ids.loser}); err != nil { + return MergeResult{}, fmt.Errorf("move contextual likes: %w", err) + } + if _, err := tq.MergeRepointPlaybackErrors(ctx, dbq.MergeRepointPlaybackErrorsParams{SurvivorID: ids.survivor, LoserID: ids.loser}); err != nil { + return MergeResult{}, fmt.Errorf("move playback errors: %w", err) + } + if _, err := tq.MergeRepointLidarrRequests(ctx, dbq.MergeRepointLidarrRequestsParams{SurvivorID: ids.survivor, LoserID: ids.loser}); err != nil { + return MergeResult{}, fmt.Errorf("move lidarr request matches: %w", err) + } + + playlists, err := tq.MergeRepointPlaylistTracks(ctx, dbq.MergeRepointPlaylistTracksParams{SurvivorID: ids.survivor, LoserID: ids.loser}) + if err != nil { + return MergeResult{}, fmt.Errorf("move playlist entries: %w", err) + } + res.PlaylistEntries += len(playlists) + for _, pl := range playlists { + plKey := syncpkg.FormatUUID(pl) + changes.playlistDelete(syncpkg.EncodePlaylistTrackID(plKey, loserKey)) + changes.playlistUpsert(syncpkg.EncodePlaylistTrackID(plKey, survivorKey)) + } + + users, err := tq.MergeCopyGeneralLikes(ctx, dbq.MergeCopyGeneralLikesParams{SurvivorID: ids.survivor, LoserID: ids.loser}) + if err != nil { + return MergeResult{}, fmt.Errorf("move likes: %w", err) + } + for _, u := range users { + userKey := syncpkg.FormatUUID(u) + likers[userKey] = struct{}{} + changes.likeDelete(syncpkg.EncodeLikeID(userKey, loserKey)) + changes.likeUpsert(syncpkg.EncodeLikeID(userKey, survivorKey)) + } + + if _, err := tq.MergeCopyTrackTags(ctx, dbq.MergeCopyTrackTagsParams{SurvivorID: ids.survivor, LoserID: ids.loser}); err != nil { + return MergeResult{}, fmt.Errorf("merge tags: %w", err) + } + if _, err := tq.MergeCopyTrackSimilarity(ctx, dbq.MergeCopyTrackSimilarityParams{SurvivorID: ids.survivor, LoserID: ids.loser}); err != nil { + return MergeResult{}, fmt.Errorf("merge similarity: %w", err) + } + if err := tq.MergeInheritTrackMbid(ctx, dbq.MergeInheritTrackMbidParams{SurvivorID: ids.survivor, LoserID: ids.loser}); err != nil { + return MergeResult{}, fmt.Errorf("inherit recording mbid: %w", err) + } + + // Everything the loser carried now sits on the survivor, so the CASCADE + // this delete sets off has nothing left to destroy. + deleted, err := tq.DeleteTrack(ctx, l.ID) + if err != nil { + return MergeResult{}, fmt.Errorf("delete merged copy: %w", err) + } + tidied, err := tidyEmptiedAlbum(ctx, tq, deleted.AlbumID) + if err != nil { + return MergeResult{}, err + } + if tidied.AlbumID != nil { + res.DeletedAlbumIDs = append(res.DeletedAlbumIDs, *tidied.AlbumID) + } + if tidied.ArtistID != nil { + res.DeletedArtistIDs = append(res.DeletedArtistIDs, *tidied.ArtistID) + } + res.Removed = append(res.Removed, mergedCopyOf(l)) + changes.trackDelete(loserKey) + } + res.Likes = len(likers) + + marked, err := tq.MarkDuplicateGroupMerged(ctx, groupID) + if err != nil { + return MergeResult{}, fmt.Errorf("mark group merged: %w", err) + } + if marked != 1 { + return MergeResult{}, ErrDuplicateGroupNotPending + } + if err := changes.log(ctx, tx); err != nil { + return MergeResult{}, err + } + if err := tx.Commit(ctx); err != nil { + return MergeResult{}, fmt.Errorf("commit merge: %w", err) + } + + // After commit, like DeleteTrackFile: a leftover art directory is only disk. + if dataDir != "" { + for _, artistID := range res.DeletedArtistIDs { + if err := coverart.CleanupArtistArt(dataDir, artistID); err != nil { + logger.Warn("duplicate merge: artist-art cleanup failed", + "artist_id", syncpkg.FormatUUID(artistID), "err", err) + } + } + } + return res, nil +} + +// splitSurvivor separates the copy to keep from the copies to remove. An +// invalid survivorID takes ProposeSurvivor's choice. +func splitSurvivor( + members []dbq.ListDuplicateGroupMergeMembersRow, survivorID pgtype.UUID, +) (dbq.ListDuplicateGroupMergeMembersRow, []dbq.ListDuplicateGroupMergeMembersRow, error) { + want := "" + if survivorID.Valid { + want = syncpkg.FormatUUID(survivorID) + } else { + cands := make([]SurvivorCandidate, len(members)) + for i, m := range members { + cands[i] = SurvivorCandidate{ + TrackID: syncpkg.FormatUUID(m.ID), FileFormat: m.FileFormat, FileSize: m.FileSize, AddedAt: m.AddedAt.Time, + } + } + want, _ = ProposeSurvivor(cands) + } + + var survivor dbq.ListDuplicateGroupMergeMembersRow + found := false + var losers []dbq.ListDuplicateGroupMergeMembersRow + for _, m := range members { + if syncpkg.FormatUUID(m.ID) == want { + survivor, found = m, true + continue + } + losers = append(losers, m) + } + if !found { + return dbq.ListDuplicateGroupMergeMembersRow{}, nil, ErrSurvivorNotInGroup + } + return survivor, losers, nil +} + +func mergedCopyOf(m dbq.ListDuplicateGroupMergeMembersRow) MergedCopy { + return MergedCopy{TrackID: m.ID, FilePath: m.FilePath, TrackMbid: m.Mbid, AlbumMbid: m.AlbumMbid} +} + +// mergeChanges collects the sync-log entries a merge owes clients' caches, each +// once: a user who liked two removed copies still gets one upsert for the +// survivor. +type mergeChanges struct { + tracks, likeDeletes, likeUpserts, playlistDeletes, playlistUpserts map[string]struct{} +} + +func addTo(set *map[string]struct{}, id string) { + if *set == nil { + *set = map[string]struct{}{} + } + (*set)[id] = struct{}{} +} + +func (c *mergeChanges) trackDelete(id string) { addTo(&c.tracks, id) } +func (c *mergeChanges) likeDelete(id string) { addTo(&c.likeDeletes, id) } +func (c *mergeChanges) likeUpsert(id string) { addTo(&c.likeUpserts, id) } +func (c *mergeChanges) playlistDelete(id string) { addTo(&c.playlistDeletes, id) } +func (c *mergeChanges) playlistUpsert(id string) { addTo(&c.playlistUpserts, id) } + +func (c *mergeChanges) log(ctx context.Context, tx pgx.Tx) error { + for _, entry := range []struct { + kind syncpkg.EntityType + ids map[string]struct{} + op syncpkg.Op + }{ + {syncpkg.EntityTrack, c.tracks, syncpkg.OpDelete}, + {syncpkg.EntityLikeTrack, c.likeDeletes, syncpkg.OpDelete}, + {syncpkg.EntityLikeTrack, c.likeUpserts, syncpkg.OpUpsert}, + {syncpkg.EntityPlaylistTrack, c.playlistDeletes, syncpkg.OpDelete}, + {syncpkg.EntityPlaylistTrack, c.playlistUpserts, syncpkg.OpUpsert}, + } { + if len(entry.ids) == 0 { + continue + } + ids := make([]string, 0, len(entry.ids)) + for id := range entry.ids { + ids = append(ids, id) + } + sort.Strings(ids) + if err := syncpkg.LogChanges(ctx, tx, entry.kind, ids, entry.op); err != nil { + return fmt.Errorf("log merge changes: %w", err) + } + } + return nil +} diff --git a/internal/library/duplicate_merge_test.go b/internal/library/duplicate_merge_test.go new file mode 100644 index 00000000..729cc942 --- /dev/null +++ b/internal/library/duplicate_merge_test.go @@ -0,0 +1,281 @@ +package library + +import ( + "context" + "errors" + "io/fs" + "os" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" +) + +// mergeFixture is a library with one duplicate pair carrying history on both +// copies, and a neighbour track for similarity edges. +type mergeFixture struct { + pool *pgxpool.Pool + keep, remove, other dbq.Track + keepPath, removePath string + groupID pgtype.UUID + alice, bob dbq.User + playlistID pgtype.UUID + removePlaylistPos int32 + aliceEarlierLikeOnRem time.Time +} + +func newMergeFixture(t *testing.T) mergeFixture { + t.Helper() + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + dir := t.TempDir() + f := mergeFixture{pool: pool} + + f.keepPath = filepath.Join(dir, "keep.flac") + f.removePath = filepath.Join(dir, "remove.mp3") + for _, p := range []string{f.keepPath, f.removePath} { + if err := os.WriteFile(p, []byte("audio"), 0o644); err != nil { + t.Fatalf("write %s: %v", p, err) + } + } + var album dbq.Album + var artist dbq.Artist + f.keep, album, artist = seedTrack(t, pool, f.keepPath) + upsert := func(title, path string) dbq.Track { + t.Helper() + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: title, AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 215000, FilePath: path, FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("track %s: %v", title, err) + } + return tr + } + f.remove = upsert("WWW (copy)", f.removePath) + f.other = upsert("Neighbour", filepath.Join(dir, "other.mp3")) + + mustExec := func(sql string, args ...any) { + t.Helper() + if _, err := pool.Exec(ctx, sql, args...); err != nil { + t.Fatalf("exec %q: %v", sql, err) + } + } + // Only the copy being removed carries a recording MBID. + mustExec(`UPDATE tracks SET mbid = 'rec-www' WHERE id = $1`, f.remove.ID) + + user := func(name string) dbq.User { + t.Helper() + u, err := q.CreateUser(ctx, dbq.CreateUserParams{ + Username: dbtest.TestUserPrefix + name, PasswordHash: "x", ApiToken: name + "-merge-token", + }) + if err != nil { + t.Fatalf("user %s: %v", name, err) + } + return u + } + f.alice, f.bob = user("merge-alice"), user("merge-bob") + + // Alice liked both copies, the removed one first; Bob liked only the removed one. + f.aliceEarlierLikeOnRem = time.Now().Add(-72 * time.Hour).UTC().Truncate(time.Microsecond) + mustExec(`INSERT INTO general_likes (user_id, track_id, liked_at) VALUES ($1, $2, $3), ($1, $4, now()), ($5, $2, now())`, + f.alice.ID, f.remove.ID, f.aliceEarlierLikeOnRem, f.keep.ID, f.bob.ID) + + now := pgtype.Timestamptz{Time: time.Now(), Valid: true} + session, err := q.InsertPlaySession(ctx, dbq.InsertPlaySessionParams{UserID: f.alice.ID, StartedAt: now}) + if err != nil { + t.Fatalf("session: %v", err) + } + for _, track := range []dbq.Track{f.remove, f.remove, f.keep} { + if _, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ + UserID: f.alice.ID, TrackID: track.ID, SessionID: session.ID, StartedAt: now, + }); err != nil { + t.Fatalf("play event: %v", err) + } + } + if _, err := q.InsertSkipEvent(ctx, dbq.InsertSkipEventParams{ + UserID: f.alice.ID, TrackID: f.remove.ID, SessionID: session.ID, SkippedAt: now, PositionMs: 1000, + }); err != nil { + t.Fatalf("skip event: %v", err) + } + + pl, err := q.CreatePlaylist(ctx, dbq.CreatePlaylistParams{UserID: f.alice.ID, Name: "merge-mix"}) + if err != nil { + t.Fatalf("playlist: %v", err) + } + f.playlistID = pl.ID + entry, err := q.AppendPlaylistTrack(ctx, dbq.AppendPlaylistTrackParams{PlaylistID: pl.ID, TrackID: f.remove.ID}) + if err != nil { + t.Fatalf("playlist entry: %v", err) + } + f.removePlaylistPos = entry.Position + + mustExec(`INSERT INTO track_tags (track_id, tag, weight) VALUES ($1, 'j-pop', 1), ($1, 'house', 0.5), ($2, 'house', 0.9)`, + f.remove.ID, f.keep.ID) + mustExec(`INSERT INTO track_similarity (track_a_id, track_b_id, score, source) VALUES + ($1, $3, 0.8, 'listenbrainz'), + ($2, $3, 0.7, 'listenbrainz'), + ($1, $2, 0.99, 'listenbrainz'), + ($3, $1, 0.6, 'musicbrainz_tag')`, f.remove.ID, f.keep.ID, f.other.ID) + + if err := pool.QueryRow(ctx, + `INSERT INTO duplicate_groups (member_key, tier) VALUES ('merge-fixture', 'exact') RETURNING id`, + ).Scan(&f.groupID); err != nil { + t.Fatalf("group: %v", err) + } + mustExec(`INSERT INTO duplicate_group_members (group_id, track_id) VALUES ($1, $2), ($1, $3)`, + f.groupID, f.keep.ID, f.remove.ID) + return f +} + +func (f mergeFixture) count(t *testing.T, sql string, args ...any) int { + t.Helper() + var n int + if err := f.pool.QueryRow(context.Background(), sql, args...).Scan(&n); err != nil { + t.Fatalf("count %q: %v", sql, err) + } + return n +} + +// The #3911 proof: after a merge, every piece of history the removed copy held +// is on the copy kept, nothing is doubled, and the removed copy — row and file — +// is gone. +func TestMergeDuplicateGroup_Integration(t *testing.T) { + f := newMergeFixture(t) + ctx := context.Background() + + res, err := MergeDuplicateGroup(ctx, f.pool, nil, "", f.groupID, f.keep.ID) + if err != nil { + t.Fatalf("merge: %v", err) + } + if len(res.Removed) != 1 || res.Removed[0].FilePath != f.removePath || res.Survivor.TrackID != f.keep.ID { + t.Fatalf("result = %+v, want the removed copy reported and the kept one as survivor", res) + } + if res.PlayEvents != 2 || res.SkipEvents != 1 || res.Likes != 2 || res.PlaylistEntries != 1 { + t.Errorf("moved = plays %d skips %d likes %d playlist %d, want 2, 1, 2, 1", + res.PlayEvents, res.SkipEvents, res.Likes, res.PlaylistEntries) + } + + if _, err := os.Stat(f.removePath); !errors.Is(err, os.ErrNotExist) { + t.Errorf("removed copy's file still on disk: %v", err) + } + if _, err := os.Stat(f.keepPath); err != nil { + t.Errorf("kept copy's file is gone: %v", err) + } + if n := f.count(t, `SELECT count(*) FROM tracks WHERE id = $1`, f.remove.ID); n != 0 { + t.Errorf("removed copy's row still exists") + } + + // Likes: one per user, Alice's dated to her earlier like. + if n := f.count(t, `SELECT count(*) FROM general_likes WHERE track_id = $1`, f.keep.ID); n != 2 { + t.Errorf("likes on the kept copy = %d, want 2 (Alice once, Bob)", n) + } + var aliceLiked time.Time + if err := f.pool.QueryRow(ctx, `SELECT liked_at FROM general_likes WHERE user_id = $1 AND track_id = $2`, + f.alice.ID, f.keep.ID).Scan(&aliceLiked); err != nil { + t.Fatalf("alice's like: %v", err) + } + if !aliceLiked.Equal(f.aliceEarlierLikeOnRem) { + t.Errorf("alice's like dated %v, want her earlier like %v", aliceLiked, f.aliceEarlierLikeOnRem) + } + + // Plays and skips move exactly: none lost, none invented. + if n := f.count(t, `SELECT count(*) FROM play_events WHERE track_id = $1`, f.keep.ID); n != 3 { + t.Errorf("plays on the kept copy = %d, want 3", n) + } + if n := f.count(t, `SELECT count(*) FROM skip_events WHERE track_id = $1`, f.keep.ID); n != 1 { + t.Errorf("skips on the kept copy = %d, want 1", n) + } + + // The playlist entry stays where it was and now plays the kept copy. + if n := f.count(t, `SELECT count(*) FROM playlist_tracks WHERE playlist_id = $1 AND position = $2 AND track_id = $3`, + f.playlistID, f.removePlaylistPos, f.keep.ID); n != 1 { + t.Errorf("playlist entry at position %d does not point at the kept copy", f.removePlaylistPos) + } + + // Tags are a union; the kept copy's own weight wins where both had the tag. + if n := f.count(t, `SELECT count(*) FROM track_tags WHERE track_id = $1`, f.keep.ID); n != 2 { + t.Errorf("tags on the kept copy = %d, want 2 (house, j-pop)", n) + } + if n := f.count(t, `SELECT count(*) FROM track_tags WHERE track_id = $1 AND tag = 'house' AND weight = 0.9`, f.keep.ID); n != 1 { + t.Errorf("the kept copy's own house weight was overwritten") + } + + // Similarity: rewritten onto the kept copy, no duplicate edge, no self-edge, + // nothing left pointing at the removed copy. + if n := f.count(t, `SELECT count(*) FROM track_similarity WHERE track_a_id = $1 AND track_b_id = $2 AND source = 'listenbrainz'`, + f.keep.ID, f.other.ID); n != 1 { + t.Errorf("listenbrainz edge keep→other = %d rows, want exactly 1", n) + } + if n := f.count(t, `SELECT count(*) FROM track_similarity WHERE track_a_id = $1 AND track_b_id = $2 AND source = 'musicbrainz_tag'`, + f.other.ID, f.keep.ID); n != 1 { + t.Errorf("musicbrainz_tag edge other→keep was not carried over") + } + if n := f.count(t, `SELECT count(*) FROM track_similarity WHERE track_a_id = track_b_id`); n != 0 { + t.Errorf("a self-edge was written") + } + + // The removed copy's recording MBID is inherited; the group is closed. + if n := f.count(t, `SELECT count(*) FROM tracks WHERE id = $1 AND mbid = 'rec-www'`, f.keep.ID); n != 1 { + t.Errorf("the kept copy did not inherit the recording MBID") + } + if n := f.count(t, `SELECT count(*) FROM duplicate_groups WHERE id = $1 AND status = 'merged' AND resolved_at IS NOT NULL`, f.groupID); n != 1 { + t.Errorf("group was not marked merged") + } + + // A second merge of the same group is refused rather than repeated. + if _, err := MergeDuplicateGroup(ctx, f.pool, nil, "", f.groupID, f.keep.ID); !errors.Is(err, ErrDuplicateGroupNotPending) { + t.Errorf("second merge err = %v, want ErrDuplicateGroupNotPending", err) + } +} + +// When the removed copy's file cannot go, nothing may change: its likes, plays +// and row stay exactly where they were, and the group stays pending. +func TestMergeDuplicateGroup_UnremovableFileChangesNothing(t *testing.T) { + f := newMergeFixture(t) + stubRemoveFile(t, func(name string) error { + return &fs.PathError{Op: "remove", Path: name, Err: syscall.EROFS} + }) + + _, err := MergeDuplicateGroup(context.Background(), f.pool, nil, "", f.groupID, f.keep.ID) + var fre *FileRemoveError + if !errors.As(err, &fre) || !fre.NotWritable() { + t.Fatalf("err = %v, want a not-writable *FileRemoveError", err) + } + if n := f.count(t, `SELECT count(*) FROM tracks WHERE id = $1`, f.remove.ID); n != 1 { + t.Errorf("the copy's row was deleted although its file was not") + } + if n := f.count(t, `SELECT count(*) FROM general_likes WHERE track_id = $1`, f.remove.ID); n != 2 { + t.Errorf("likes on the copy = %d, want both still there", n) + } + if n := f.count(t, `SELECT count(*) FROM play_events WHERE track_id = $1`, f.remove.ID); n != 2 { + t.Errorf("plays on the copy = %d, want both still there", n) + } + if n := f.count(t, `SELECT count(*) FROM duplicate_groups WHERE id = $1 AND status = 'pending'`, f.groupID); n != 1 { + t.Errorf("group left pending = false, want it still pending") + } +} + +func TestMergeDuplicateGroup_SurvivorMustBeAMember(t *testing.T) { + f := newMergeFixture(t) + var stranger pgtype.UUID + stranger.Bytes[15], stranger.Valid = 0xEE, true + + _, err := MergeDuplicateGroup(context.Background(), f.pool, nil, "", f.groupID, stranger) + if !errors.Is(err, ErrSurvivorNotInGroup) { + t.Fatalf("err = %v, want ErrSurvivorNotInGroup", err) + } + if _, err := os.Stat(f.removePath); err != nil { + t.Errorf("a refused merge removed a file: %v", err) + } + if n := f.count(t, `SELECT count(*) FROM duplicate_groups WHERE id = $1 AND status = 'pending'`, f.groupID); n != 1 { + t.Errorf("a refused merge changed the group") + } +} diff --git a/internal/library/duplicate_survivor.go b/internal/library/duplicate_survivor.go new file mode 100644 index 00000000..6f90e64f --- /dev/null +++ b/internal/library/duplicate_survivor.go @@ -0,0 +1,77 @@ +package library + +import ( + "sort" + "strings" + "time" +) + +// SurvivorCandidate is what choosing which copy to keep needs to know about one +// member of a duplicate group. +type SurvivorCandidate struct { + TrackID string + FileFormat string + FileSize int64 + AddedAt time.Time +} + +// losslessFormats are the scanned extensions that are lossless by definition. +// m4a is left out on purpose: it holds either ALAC or AAC, and the scanner +// records only the extension, so calling it lossless would sometimes prefer an +// AAC copy over a FLAC one. +var losslessFormats = map[string]bool{"flac": true, "wav": true} + +// ProposeSurvivor picks which copy of a duplicate group to keep, and gives the +// reason in words the operator reads beside it. It is a default, not a verdict: +// the report shows it and the merge (#3911) lets the operator choose another. +// +// In order: +// 1. lossless over lossy — the one difference no later step can recover +// 2. the larger file — for one recording at one duration that is the higher +// bitrate. The scanner does not record bitrate (tracks.bitrate is never +// filled), so file size is the signal that actually exists +// 3. the copy in the library longest — the one most likely to carry the play +// history and likes, so the merge moves the least +// 4. the lowest track id, so the choice is stable between page loads +func ProposeSurvivor(cands []SurvivorCandidate) (trackID, reason string) { + if len(cands) == 0 { + return "", "" + } + ranked := append([]SurvivorCandidate(nil), cands...) + sort.SliceStable(ranked, func(i, j int) bool { return survivorBefore(ranked[i], ranked[j]) }) + best := ranked[0] + if len(ranked) == 1 { + return best.TrackID, "the only copy" + } + + // The reason names the first rule that separated the best copy from the + // runner-up — the rule that actually decided, not every rule it passed. + next := ranked[1] + switch { + case isLossless(best) != isLossless(next): + return best.TrackID, "lossless (" + strings.ToLower(best.FileFormat) + ")" + case best.FileSize != next.FileSize: + return best.TrackID, "largest file" + case !best.AddedAt.Equal(next.AddedAt): + return best.TrackID, "in the library longest" + default: + return best.TrackID, "copies are otherwise identical" + } +} + +func survivorBefore(a, b SurvivorCandidate) bool { + if isLossless(a) != isLossless(b) { + return isLossless(a) + } + if a.FileSize != b.FileSize { + return a.FileSize > b.FileSize + } + if !a.AddedAt.Equal(b.AddedAt) { + return a.AddedAt.Before(b.AddedAt) + } + return a.TrackID < b.TrackID +} + +func isLossless(c SurvivorCandidate) bool { + return losslessFormats[strings.ToLower(c.FileFormat)] +} diff --git a/internal/library/duplicate_survivor_test.go b/internal/library/duplicate_survivor_test.go new file mode 100644 index 00000000..4e770d7e --- /dev/null +++ b/internal/library/duplicate_survivor_test.go @@ -0,0 +1,91 @@ +package library + +import ( + "testing" + "time" +) + +func TestProposeSurvivor(t *testing.T) { + older := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + newer := older.Add(24 * time.Hour) + cases := []struct { + name string + cands []SurvivorCandidate + wantID string + wantReason string + }{ + { + // Lossless wins even against a much larger lossy file, and even + // when the lossy copy has been in the library longer. + name: "lossless beats larger and older", + cands: []SurvivorCandidate{ + {TrackID: "mp3", FileFormat: "mp3", FileSize: 90_000_000, AddedAt: older}, + {TrackID: "flac", FileFormat: "FLAC", FileSize: 30_000_000, AddedAt: newer}, + }, + wantID: "flac", wantReason: "lossless (flac)", + }, + { + // m4a may be AAC; it must not outrank an mp3 just for being m4a. + name: "m4a is not treated as lossless", + cands: []SurvivorCandidate{ + {TrackID: "m4a", FileFormat: "m4a", FileSize: 5_000_000, AddedAt: older}, + {TrackID: "mp3", FileFormat: "mp3", FileSize: 9_000_000, AddedAt: newer}, + }, + wantID: "mp3", wantReason: "largest file", + }, + { + name: "larger file wins among lossy copies", + cands: []SurvivorCandidate{ + {TrackID: "128k", FileFormat: "mp3", FileSize: 3_400_000, AddedAt: older}, + {TrackID: "320k", FileFormat: "mp3", FileSize: 8_600_000, AddedAt: newer}, + }, + wantID: "320k", wantReason: "largest file", + }, + { + // The #3885 pair: identical audio, sizes equal but for the tags. + name: "the longest-standing copy wins when size ties", + cands: []SurvivorCandidate{ + {TrackID: "www-02", FileFormat: "mp3", FileSize: 6_900_000, AddedAt: newer}, + {TrackID: "www-01", FileFormat: "mp3", FileSize: 6_900_000, AddedAt: older}, + }, + wantID: "www-01", wantReason: "in the library longest", + }, + { + name: "a full tie falls back to the lowest id, stably", + cands: []SurvivorCandidate{ + {TrackID: "b", FileFormat: "mp3", FileSize: 1, AddedAt: older}, + {TrackID: "a", FileFormat: "mp3", FileSize: 1, AddedAt: older}, + }, + wantID: "a", wantReason: "copies are otherwise identical", + }, + { + name: "one copy", + cands: []SurvivorCandidate{{TrackID: "only", FileFormat: "mp3", FileSize: 1, AddedAt: older}}, + wantID: "only", wantReason: "the only copy", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + id, reason := ProposeSurvivor(tc.cands) + if id != tc.wantID || reason != tc.wantReason { + t.Fatalf("ProposeSurvivor = (%q, %q), want (%q, %q)", id, reason, tc.wantID, tc.wantReason) + } + }) + } +} + +// The reason must name the rule that decided. Across three copies that is the +// comparison between first and second place, not the first rule any pair +// differs on: here the lossy copy differs from the others by format, but the +// two FLACs are separated by size. +func TestProposeSurvivor_ReasonIsTheDecidingRule(t *testing.T) { + at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + id, reason := ProposeSurvivor([]SurvivorCandidate{ + {TrackID: "mp3", FileFormat: "mp3", FileSize: 99_000_000, AddedAt: at}, + {TrackID: "flac-small", FileFormat: "flac", FileSize: 20_000_000, AddedAt: at}, + {TrackID: "flac-big", FileFormat: "flac", FileSize: 40_000_000, AddedAt: at}, + }) + if id != "flac-big" || reason != "largest file" { + t.Fatalf("got (%q, %q), want (flac-big, largest file)", id, reason) + } +} diff --git a/internal/library/duplicate_sweep.go b/internal/library/duplicate_sweep.go new file mode 100644 index 00000000..34dd2cb8 --- /dev/null +++ b/internal/library/duplicate_sweep.go @@ -0,0 +1,398 @@ +package library + +import ( + "context" + "errors" + "fmt" + "log/slog" + "sort" + "strings" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" +) + +// Duplicate sweep (M400 #3910). +// +// Reads fingerprints, runs them through the matcher and records what it proposes +// in duplicate_groups. It never merges or deletes anything the operator has not +// asked for: a group is a proposal, reviewed in the admin report (#3912). +// +// Blocked on duration, deliberately not on title: the #3885 pair are titled +// "WWW" and "WWW (instrumental)", so a title block would have missed the case +// that started the milestone. Candidates stream in (duration_ms, id) order and +// the grouper holds only a few seconds of durations at a time. + +// duplicateCandidatePage is how many candidates one query returns. Each row +// carries a ~4 KB fingerprint, so a page is about 2 MB. +const duplicateCandidatePage = 500 + +// duplicateSweepTick is how often the worker checks whether a sweep is due. The +// operator's sweep interval (#3913) is the least time between sweeps; the tick +// only bounds how late past it one starts. With nothing due, a tick is two cheap +// aggregate queries. +const duplicateSweepTick = time.Hour + +// sweepIntervalSlack absorbs the moment between a tick and the sweep it starts +// stamping started_at. Without it a one-hour interval checked on a one-hour tick +// would find the last sweep a moment under an hour old, and skip every other tick. +const sweepIntervalSlack = 5 * time.Minute + +// staleDuplicateSweepThreshold is the age past which an in-flight sweep is +// assumed dead — a crash mid-sweep leaves finished_at NULL for ever — and another +// may start. Twice the library scan's threshold, because a sweep compares +// fingerprints across the whole library and can legitimately run long on a big +// one. +const staleDuplicateSweepThreshold = 2 * time.Hour + +// duplicateSweepFinishTimeout bounds recording that a sweep ended. It runs on a +// context detached from the sweep's own, so a sweep cancelled at shutdown still +// closes its row rather than leaving it in flight until the reaper. +const duplicateSweepFinishTimeout = 10 * time.Second + +// DuplicateSweepResult tallies one sweep. +type DuplicateSweepResult struct { + Candidates int // tracks with a chromaprint that were streamed + Groups int // groups the matcher found + Proposed int // written as pending, new or refreshed + Suppressed int // not proposed: already dismissed or resolved by the operator + Retired int // pending proposals this sweep did not find again, removed + Oversize int // acoustic clusters too large to propose +} + +// RunDuplicateSweep runs one sweep and records it in duplicate_sweeps. cfg is a +// snapshot: one sweep applies one threshold and one length throughout. +func RunDuplicateSweep( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, cfg FingerprintSettings, +) (DuplicateSweepResult, error) { + return runDuplicateSweep(ctx, pool, logger, cfg, duplicateCandidatePage) +} + +func runDuplicateSweep( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, cfg FingerprintSettings, pageSize int32, +) (DuplicateSweepResult, error) { + q := dbq.New(pool) + sweep, err := q.StartDuplicateSweep(ctx) + if err != nil { + return DuplicateSweepResult{}, fmt.Errorf("start duplicate sweep: %w", err) + } + + res, runErr := sweepDuplicates(ctx, q, sweep.ID, cfg, pageSize) + + finishCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), duplicateSweepFinishTimeout) + defer cancel() + errMsg := "" + if runErr != nil { + errMsg = runErr.Error() + } + candidates, groups, oversize := int32(res.Candidates), int32(res.Groups), int32(res.Oversize) + if ferr := q.FinishDuplicateSweep(finishCtx, dbq.FinishDuplicateSweepParams{ + ID: sweep.ID, Candidates: &candidates, GroupsFound: &groups, OversizeClusters: &oversize, + ErrorMessage: errMsg, + }); ferr != nil { + logger.Error("duplicate sweep: recording the end of the sweep failed", "err", ferr) + if runErr == nil { + runErr = fmt.Errorf("finish duplicate sweep: %w", ferr) + } + } + + logger.Info("duplicate sweep complete", + "candidates", res.Candidates, "groups", res.Groups, "proposed", res.Proposed, + "suppressed", res.Suppressed, "retired", res.Retired, "oversize", res.Oversize, "err", runErr) + return res, runErr +} + +func sweepDuplicates( + ctx context.Context, q *dbq.Queries, sweepID pgtype.UUID, cfg FingerprintSettings, pageSize int32, +) (DuplicateSweepResult, error) { + var res DuplicateSweepResult + + // Exact tier, library-wide, in one query. + exactRows, err := q.ListExactDuplicateHashes(ctx, fingerprintVersion) + if err != nil { + return res, fmt.Errorf("list exact duplicates: %w", err) + } + exactMembers := make([][]string, len(exactRows)) + exactOf := map[string]int{} + for i, row := range exactRows { + exactMembers[i] = formatUUIDs(row.TrackIds) + for _, id := range exactMembers[i] { + exactOf[id] = i + } + } + exactSeen := make([]bool, len(exactRows)) + + // Acoustic tier, streamed in duration order. The first member of an exact + // group the stream meets stands in for the whole group; the rest are skipped. + // Only prints at the current length are streamed: a print at another length + // cannot be compared, and is waiting on the backfill to be re-derived. + grouper := newStreamGrouper(cfg.AcousticMaxBitErrorRate) + params := dbq.ListDuplicateCandidatesParams{ + CurrentVersion: fingerprintVersion, + ChromaprintLengthSec: cfg.ChromaprintLengthSec, + // Durations are never negative, and the all-zero uuid sorts first: every + // row is after this cursor. Valid must be true, or "> NULL" matches nothing. + AfterDurationMs: -1, + AfterID: pgtype.UUID{Valid: true}, + PageLimit: pageSize, + } + for { + if err := ctx.Err(); err != nil { + return res, err + } + rows, err := q.ListDuplicateCandidates(ctx, params) + if err != nil { + return res, fmt.Errorf("list duplicate candidates: %w", err) + } + for _, row := range rows { + res.Candidates++ + id := syncpkg.FormatUUID(row.ID) + unit := &groupUnit{ids: []string{id}, durationMs: row.DurationMs, sortKey: id} + if gi, ok := exactOf[id]; ok { + if exactSeen[gi] { + continue + } + exactSeen[gi] = true + unit.ids, unit.exact = exactMembers[gi], true + } + unit.print = preparePrint(row.Chromaprint) + grouper.push(unit) + } + if int32(len(rows)) < pageSize { + break + } + last := rows[len(rows)-1] + params.AfterDurationMs, params.AfterID = last.DurationMs, last.ID + } + + found := grouper.finish() + // Exact groups none of whose members has a chromaprint never reached the + // stream. Identical bytes need no acoustic evidence. + for gi, seen := range exactSeen { + if !seen { + found.Groups = append(found.Groups, duplicateGroup{Tier: tierExact, Members: exactMembers[gi]}) + } + } + sortGroups(found.Groups) + res.Groups, res.Oversize = len(found.Groups), found.OversizeClusters + + dismissed, err := q.ListDismissedDuplicateMemberSets(ctx) + if err != nil { + return res, fmt.Errorf("list dismissed duplicate groups: %w", err) + } + dismissedSets := make([]map[string]struct{}, 0, len(dismissed)) + for _, d := range dismissed { + set := map[string]struct{}{} + for _, id := range formatUUIDs(d.TrackIds) { + set[id] = struct{}{} + } + dismissedSets = append(dismissedSets, set) + } + + for _, group := range found.Groups { + if coveredByDismissal(group.Members, dismissedSets) { + res.Suppressed++ + continue + } + up := dbq.UpsertDuplicateGroupParams{ + MemberKey: strings.Join(group.Members, ","), + Tier: string(group.Tier), + SweepID: sweepID, + } + if group.Tier == tierAcoustic { + worst := float32(group.WorstBitErrorRate) + up.WorstBitErrorRate = &worst + } + groupID, err := q.UpsertDuplicateGroup(ctx, up) + if errors.Is(err, pgx.ErrNoRows) { + // This exact member set was already dismissed or merged. + res.Suppressed++ + continue + } + if err != nil { + return res, fmt.Errorf("upsert duplicate group: %w", err) + } + for _, id := range group.Members { + var trackID pgtype.UUID + if err := trackID.Scan(id); err != nil { + return res, fmt.Errorf("parse track id %q: %w", id, err) + } + if err := q.AddDuplicateGroupMember(ctx, dbq.AddDuplicateGroupMemberParams{ + GroupID: groupID, TrackID: trackID, + }); err != nil { + return res, fmt.Errorf("add duplicate group member: %w", err) + } + } + res.Proposed++ + } + + // Only after a complete sweep: a sweep that failed partway has no basis for + // concluding that anything it did not reach has gone. + retired, err := q.DeleteStalePendingDuplicateGroups(ctx, sweepID) + if err != nil { + return res, fmt.Errorf("retire stale duplicate groups: %w", err) + } + res.Retired = int(retired) + return res, nil +} + +// coveredByDismissal reports whether every member of a proposal sat together in +// one group the operator dismissed. A subset counts: dismissing {A, B, C} said +// none of them are copies of each other, so proposing {A, B} again would be +// asking the same question twice. A superset does not count: a new copy joining +// is new evidence, and worth asking about. +func coveredByDismissal(members []string, dismissed []map[string]struct{}) bool { + for _, set := range dismissed { + covered := true + for _, id := range members { + if _, ok := set[id]; !ok { + covered = false + break + } + } + if covered { + return true + } + } + return false +} + +func formatUUIDs(ids []pgtype.UUID) []string { + out := make([]string, len(ids)) + for i, id := range ids { + out[i] = syncpkg.FormatUUID(id) + } + sort.Strings(out) + return out +} + +// TryStartDuplicateSweep starts a sweep in the background unless one is already +// running, reaping a sweep that has been in flight past +// staleDuplicateSweepThreshold. Mirrors TryStartScan. The sweep runs on ctx, so +// a caller answering an HTTP request must pass a context that outlives it. +func TryStartDuplicateSweep( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, cfg FingerprintSettings, +) (bool, error) { + q := dbq.New(pool) + row, err := q.GetInFlightDuplicateSweep(ctx) + switch { + case err == nil: + age := time.Since(row.StartedAt.Time) + if age <= staleDuplicateSweepThreshold { + return false, nil + } + logger.Warn("reaping stale duplicate sweep", "id", syncpkg.FormatUUID(row.ID), "age", age) + if ferr := q.FinishDuplicateSweep(ctx, dbq.FinishDuplicateSweepParams{ + ID: row.ID, ErrorMessage: "reaped (stale)", + }); ferr != nil { + return false, fmt.Errorf("reap stale duplicate sweep: %w", ferr) + } + case !errors.Is(err, pgx.ErrNoRows): + return false, fmt.Errorf("duplicate sweep in-flight check: %w", err) + } + + go func() { + if _, err := RunDuplicateSweep(ctx, pool, logger, cfg); err != nil { + logger.Warn("duplicate sweep failed", "err", err) + } + }() + return true, nil +} + +// DuplicateSweepWorker sweeps whenever its input has changed, at most once per +// the operator's sweep interval. +type DuplicateSweepWorker struct { + pool *pgxpool.Pool + logger *slog.Logger + settings *FingerprintSettingsService + tick time.Duration +} + +// NewDuplicateSweepWorker builds a worker with the production cadence. settings +// is shared with the admin API; nil runs on defaults. +func NewDuplicateSweepWorker( + pool *pgxpool.Pool, logger *slog.Logger, settings *FingerprintSettingsService, +) *DuplicateSweepWorker { + return &DuplicateSweepWorker{pool: pool, logger: logger, settings: settings, tick: duplicateSweepTick} +} + +// Run blocks until ctx is cancelled, checking once at start and then each tick. +func (w *DuplicateSweepWorker) Run(ctx context.Context) { + w.tickOnce(ctx) + t := time.NewTicker(w.tick) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + w.tickOnce(ctx) + } + } +} + +// tickOnce contains one check so nothing it does can stop the next tick (rule 157). +func (w *DuplicateSweepWorker) tickOnce(ctx context.Context) { + defer func() { + if r := recover(); r != nil { + w.logger.Error("duplicate sweep: tick panicked", "panic", r) + } + }() + cfg := w.settings.Get() + due, err := duplicateSweepDue(ctx, dbq.New(w.pool), cfg, time.Now()) + if err != nil { + if ctx.Err() == nil { + w.logger.Warn("duplicate sweep: due check failed", "err", err) + } + return + } + if !due { + return + } + if _, err := TryStartDuplicateSweep(ctx, w.pool, w.logger, cfg); err != nil { + w.logger.Warn("duplicate sweep: start failed", "err", err) + } +} + +// duplicateSweepDue reads what sweepIsDue decides on. +func duplicateSweepDue(ctx context.Context, q *dbq.Queries, cfg FingerprintSettings, now time.Time) (bool, error) { + latest, err := q.GetLatestFingerprintComputedAt(ctx) + if err != nil { + return false, fmt.Errorf("latest fingerprint: %w", err) + } + var lastStart pgtype.Timestamptz + last, err := q.GetLatestDuplicateSweep(ctx) + switch { + case err == nil: + lastStart = last.StartedAt + case !errors.Is(err, pgx.ErrNoRows): + return false, fmt.Errorf("latest duplicate sweep: %w", err) + } + return sweepIsDue(latest, lastStart, cfg, now), nil +} + +// sweepIsDue reports whether a sweep should start: something it reads has +// changed since the last sweep started, and the operator's interval has passed. +// +// Two things can change its answer. Fingerprints are its input, so any written +// after the last sweep began count; while the backfill runs that is true every +// tick, which is what the interval is for. And a settings save counts, because a +// new threshold or length changes what the same fingerprints group into. +func sweepIsDue(latestFingerprint, lastSweepStart pgtype.Timestamptz, cfg FingerprintSettings, now time.Time) bool { + if !latestFingerprint.Valid { + return false // nothing fingerprinted yet + } + if !lastSweepStart.Valid { + return true // never swept + } + interval := time.Duration(cfg.SweepIntervalHours) * time.Hour + if now.Sub(lastSweepStart.Time) < interval-sweepIntervalSlack { + return false + } + return latestFingerprint.Time.After(lastSweepStart.Time) || cfg.UpdatedAt.After(lastSweepStart.Time) +} diff --git a/internal/library/duplicate_sweep_test.go b/internal/library/duplicate_sweep_test.go new file mode 100644 index 00000000..15bc478b --- /dev/null +++ b/internal/library/duplicate_sweep_test.go @@ -0,0 +1,250 @@ +package library + +import ( + "bytes" + "context" + "io" + "log/slog" + "path/filepath" + "sort" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" +) + +func TestCoveredByDismissal(t *testing.T) { + dismissed := []map[string]struct{}{{"a": {}, "b": {}, "c": {}}} + for _, tc := range []struct { + name string + members []string + want bool + }{ + {"the same set", []string{"a", "b", "c"}, true}, + {"a subset of it", []string{"a", "b"}, true}, + // A new copy joining is new evidence: ask again. + {"a superset of it", []string{"a", "b", "c", "d"}, false}, + {"overlapping only in part", []string{"a", "d"}, false}, + {"unrelated", []string{"x", "y"}, false}, + } { + if got := coveredByDismissal(tc.members, dismissed); got != tc.want { + t.Errorf("%s: coveredByDismissal = %v, want %v", tc.name, got, tc.want) + } + } +} + +// TestDuplicateSweep_Integration pins what the sweep proposes, what it leaves out, +// and how re-sweeping treats a dismissal and a proposal that no longer holds. +func TestDuplicateSweep_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + dir := t.TempDir() + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + // seedTrack's own track has no fingerprint row: it must be absent from the + // report, not grouped with every other track lacking one. + _, album, artist := seedTrack(t, pool, filepath.Join(dir, "unfingerprinted.mp3")) + hash := func(b byte) []byte { return bytes.Repeat([]byte{b}, 32) } + add := func(name string, durationMs int32, sum []byte, print []int32) string { + t.Helper() + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: name, AlbumID: album.ID, ArtistID: artist.ID, DurationMs: durationMs, + FilePath: filepath.Join(dir, name+".mp3"), FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("track %s: %v", name, err) + } + if err := q.UpsertTrackFingerprint(ctx, dbq.UpsertTrackFingerprintParams{ + TrackID: tr.ID, AudioStreamSha256: sum, Chromaprint: print, FingerprintVersion: fingerprintVersion, + ChromaprintLengthSec: defaultChromaprintLengthSec, + }); err != nil { + t.Fatalf("fingerprint %s: %v", name, err) + } + return syncpkg.FormatUUID(tr.ID) + } + key := func(ids ...string) string { + sorted := append([]string(nil), ids...) + sort.Strings(sorted) + return strings.Join(sorted, ",") + } + + recording := randomPrint(200, printLen) + onAlbum := add("recording-album", 240000, hash(1), recording) + onCompilation := add("recording-compilation", 241000, hash(2), withBitNoise(recording, 0.05, 201)) + www1 := add("www-01", 215000, hash(9), randomPrint(210, printLen)) + www2 := add("www-02", 215000, hash(9), randomPrint(210, printLen)) + // Near-identical duration to the recording, different audio. + add("different-song", 240500, hash(3), randomPrint(220, printLen)) + // Identical to the album copy, but its file is gone: nothing to compare. + missing := add("missing-copy", 240000, hash(4), recording) + if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() WHERE file_path LIKE '%missing-copy.mp3'"); err != nil { + t.Fatalf("mark missing: %v", err) + } + + type stored struct { + tier, status string + } + groups := func() map[string]stored { + t.Helper() + rows, err := pool.Query(ctx, `SELECT member_key, tier, status FROM duplicate_groups`) + if err != nil { + t.Fatalf("read groups: %v", err) + } + defer rows.Close() + out := map[string]stored{} + for rows.Next() { + var k string + var s stored + if err := rows.Scan(&k, &s.tier, &s.status); err != nil { + t.Fatalf("scan group: %v", err) + } + out[k] = s + } + return out + } + + // 1. A page size of one forces the keyset cursor across every candidate. + res, err := runDuplicateSweep(ctx, pool, logger, DefaultFingerprintSettings, 1) + if err != nil { + t.Fatalf("first sweep: %v", err) + } + // Five tracks carry a chromaprint and a present file. + if res.Candidates != 5 || res.Groups != 2 || res.Proposed != 2 { + t.Fatalf("first sweep = %+v, want 5 candidates, 2 groups, 2 proposed", res) + } + acousticKey, exactKey := key(onAlbum, onCompilation), key(www1, www2) + got := groups() + want := map[string]stored{ + acousticKey: {"acoustic", "pending"}, + exactKey: {"exact", "pending"}, + } + if len(got) != len(want) || got[acousticKey] != want[acousticKey] || got[exactKey] != want[exactKey] { + t.Fatalf("groups = %+v, want %+v", got, want) + } + for k := range got { + if strings.Contains(k, missing) { + t.Fatalf("a missing track was proposed: %s", k) + } + } + + // 2. A dismissed group is not proposed again, and the pending one is + // refreshed in place rather than duplicated. + if _, err := pool.Exec(ctx, "UPDATE duplicate_groups SET status = 'dismissed' WHERE member_key = $1", acousticKey); err != nil { + t.Fatalf("dismiss: %v", err) + } + res, err = runDuplicateSweep(ctx, pool, logger, DefaultFingerprintSettings, duplicateCandidatePage) + if err != nil { + t.Fatalf("second sweep: %v", err) + } + if res.Proposed != 1 || res.Suppressed != 1 { + t.Fatalf("second sweep = %+v, want 1 proposed, 1 suppressed", res) + } + got = groups() + if len(got) != 2 || got[acousticKey].status != "dismissed" || got[exactKey].status != "pending" { + t.Fatalf("after dismissal groups = %+v, want the dismissal kept and one pending group", got) + } + + // 3. A proposal that no longer holds is retired; the dismissal survives it. + if _, err := pool.Exec(ctx, + "DELETE FROM track_fingerprints f USING tracks t WHERE f.track_id = t.id AND t.file_path LIKE '%www-02.mp3'"); err != nil { + t.Fatalf("drop fingerprint: %v", err) + } + res, err = runDuplicateSweep(ctx, pool, logger, DefaultFingerprintSettings, duplicateCandidatePage) + if err != nil { + t.Fatalf("third sweep: %v", err) + } + if res.Retired != 1 { + t.Fatalf("third sweep = %+v, want 1 retired", res) + } + got = groups() + if len(got) != 1 || got[acousticKey].status != "dismissed" { + t.Fatalf("after retiring groups = %+v, want only the dismissal", got) + } + + // 4. The sweep record reflects the last run. + last, err := q.GetLatestDuplicateSweep(ctx) + if err != nil { + t.Fatalf("latest sweep: %v", err) + } + if !last.FinishedAt.Valid || last.ErrorMessage != nil { + t.Fatalf("latest sweep = %+v, want finished without error", last) + } + + // 5. Prints taken at another length are never compared (#3913). Every print + // here was taken at the default length, so a sweep at 60s has nothing to read, + // rather than scoring 120s prints against each other as if they were 60s ones. + atOtherLength := DefaultFingerprintSettings + atOtherLength.ChromaprintLengthSec = 60 + res, err = runDuplicateSweep(ctx, pool, logger, atOtherLength, duplicateCandidatePage) + if err != nil { + t.Fatalf("sweep at another length: %v", err) + } + if res.Candidates != 0 || res.Groups != 0 { + t.Fatalf("sweep at another length = %+v, want no candidates and no groups", res) + } + + // 6. The sweep applies the threshold it is given. The recording's two copies + // disagree on about 5% of their bits: grouped at the default, not at 1%. + if _, err := pool.Exec(ctx, "DELETE FROM duplicate_groups"); err != nil { + t.Fatalf("clear groups: %v", err) + } + strict := DefaultFingerprintSettings + strict.AcousticMaxBitErrorRate = 0.01 + res, err = runDuplicateSweep(ctx, pool, logger, strict, duplicateCandidatePage) + if err != nil { + t.Fatalf("strict sweep: %v", err) + } + if res.Groups != 0 { + t.Fatalf("sweep at a 1%% threshold = %+v, want the copies 5%% apart left ungrouped", res) + } + res, err = runDuplicateSweep(ctx, pool, logger, DefaultFingerprintSettings, duplicateCandidatePage) + if err != nil { + t.Fatalf("default sweep: %v", err) + } + if got := groups(); res.Groups != 1 || got[acousticKey] != (stored{"acoustic", "pending"}) { + t.Fatalf("sweep at the default threshold = %+v, groups %+v; want the recording's copies proposed", res, got) + } +} + +func TestSweepIsDue(t *testing.T) { + now := time.Date(2026, 9, 11, 12, 0, 0, 0, time.UTC) + at := func(ago time.Duration) pgtype.Timestamptz { + return pgtype.Timestamptz{Time: now.Add(-ago), Valid: true} + } + never := pgtype.Timestamptz{} + hourly := DefaultFingerprintSettings + daily := DefaultFingerprintSettings + daily.SweepIntervalHours = 24 + savedAgo := func(ago time.Duration) FingerprintSettings { + s := DefaultFingerprintSettings + s.UpdatedAt = now.Add(-ago) + return s + } + for _, tc := range []struct { + name string + latestPrint, lastSweep pgtype.Timestamptz + cfg FingerprintSettings + want bool + }{ + {"nothing fingerprinted", never, never, hourly, false}, + {"never swept", at(time.Minute), never, hourly, true}, + {"new fingerprints since the last sweep", at(10 * time.Minute), at(2 * time.Hour), hourly, true}, + {"nothing new since the last sweep", at(3 * time.Hour), at(2 * time.Hour), hourly, false}, + // The sweep started a moment after the previous tick, so one tick later + // it is a moment under an hour old. Without the slack this is false. + {"one tick after an hourly sweep", at(time.Minute), at(time.Hour - 2*time.Second), hourly, true}, + {"new fingerprints inside the interval", at(time.Minute), at(3 * time.Hour), daily, false}, + {"new fingerprints past the interval", at(time.Minute), at(25 * time.Hour), daily, true}, + {"settings saved since the last sweep", at(3 * time.Hour), at(2 * time.Hour), savedAgo(time.Hour), true}, + {"settings saved before the last sweep", at(3 * time.Hour), at(2 * time.Hour), savedAgo(4 * time.Hour), false}, + } { + if got := sweepIsDue(tc.latestPrint, tc.lastSweep, tc.cfg, now); got != tc.want { + t.Errorf("%s: sweepIsDue = %v, want %v", tc.name, got, tc.want) + } + } +} diff --git a/internal/library/fingerprint.go b/internal/library/fingerprint.go index ebd6b3db..4a33117d 100644 --- a/internal/library/fingerprint.go +++ b/internal/library/fingerprint.go @@ -50,21 +50,28 @@ const fingerprintTimeout = 60 * time.Second const fingerprintWaitDelay = 5 * time.Second // fingerprintVersion stamps how a track_fingerprints row was derived. Bump it -// whenever the derivation changes — the hash arguments, fpcalc's flags or its -// length — and the backfill re-derives every row below it. Fingerprints taken -// by two methods are not comparable, and nothing else would reveal that the -// library held a mix. +// whenever the derivation changes — the hash arguments or fpcalc's flags — and +// the backfill re-derives every row below it. Fingerprints taken by two methods +// are not comparable, and nothing else would reveal that the library held a mix. +// +// The length is deliberately not part of it: it is an operator setting (#3913), +// so each row records the length it was taken at and readers compare only rows +// at the current one. See fingerprint_settings.go. const fingerprintVersion int16 = 1 // errFingerprintTimeout marks a tool that ran out of time. Distinct from a // failed exit because a stall is a fact about the mount, not about the file. var errFingerprintTimeout = errors.New("fingerprint tool timed out") -// defaultChromaprintLengthSec is how many seconds of audio fpcalc fingerprints. -// 120 is fpcalc's own default. Fingerprints taken at different lengths are not -// comparable, so changing this has to re-derive every stored one. +// defaultChromaprintLengthSec is the shipped value of the length setting (#3913): +// how many seconds of audio fpcalc fingerprints. 120 is fpcalc's own default. const defaultChromaprintLengthSec = 120 +// errChromaprintSkipped marks a chromaprint not taken because fingerprinting is +// switched off. Inconclusive rather than a verdict: nothing was learned about +// the file. +var errChromaprintSkipped = errors.New("chromaprint skipped: fingerprinting is off") + // fpcalcStderrTail caps how much of a failing tool's stderr reaches the log. const fpcalcStderrTail = 512 @@ -113,11 +120,24 @@ type fingerprintResult struct { printErr error } -// computeFingerprint derives both halves for the file at path. -func computeFingerprint(ctx context.Context, path string) fingerprintResult { +// fingerprintOptions is what the settings decide for one attempt. Captured once +// per file, so the length a chromaprint was taken at is the length stored with it +// even if the setting changes mid-attempt. +type fingerprintOptions struct { + lengthSec int32 + // chromaprint false takes the stream hash alone: a demux, no decode. + chromaprint bool +} + +// computeFingerprint derives the halves opts asks for, for the file at path. +func computeFingerprint(ctx context.Context, path string, opts fingerprintOptions) fingerprintResult { var r fingerprintResult r.streamSHA256, r.hashErr = computeAudioStreamSHA256(ctx, path) - r.chromaprint, r.printErr = computeChromaprint(ctx, path, defaultChromaprintLengthSec) + if !opts.chromaprint { + r.printErr = errChromaprintSkipped + return r + } + r.chromaprint, r.printErr = computeChromaprint(ctx, path, opts.lengthSec) return r } @@ -130,11 +150,13 @@ func (r fingerprintResult) inconclusive() bool { } // isInconclusive names the failures that are not a verdict on the file: a -// stall, a cancelled scan, and a tool that is not installed. The last matters -// outside the image — a dev binary run without fpcalc on PATH must not stamp -// every track in the library as unfingerprintable. +// stall, a cancelled scan, a tool that is not installed, and a chromaprint +// skipped because fingerprinting is off. A missing tool matters outside the +// image — a dev binary run without fpcalc on PATH must not stamp every track in +// the library as unfingerprintable. func isInconclusive(err error) bool { return errors.Is(err, errFingerprintTimeout) || + errors.Is(err, errChromaprintSkipped) || errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, exec.ErrNotFound) @@ -142,11 +164,11 @@ func isInconclusive(err error) bool { // fingerprintFile runs the scanner's fingerprinter. A Scanner built without New // gets the real tools rather than a nil-func panic halfway through a scan. -func (s *Scanner) fingerprintFile(ctx context.Context, path string) fingerprintResult { +func (s *Scanner) fingerprintFile(ctx context.Context, path string, opts fingerprintOptions) fingerprintResult { if s.fingerprint == nil { - return computeFingerprint(ctx, path) + return computeFingerprint(ctx, path, opts) } - return s.fingerprint(ctx, path) + return s.fingerprint(ctx, path, opts) } // fingerprintOutcome is what storeFingerprint did with one attempt. @@ -163,9 +185,11 @@ const ( // the backfill (#3908) alike, so there is one rule for what gets written. It // never fails its caller: a missing fingerprint only keeps a track out of // duplicate detection, which is not worth dropping a scan or a pass over. +// +// lengthSec is the length fp's chromaprint was taken at, stored with it (#3913). func storeFingerprint( ctx context.Context, q *dbq.Queries, logger *slog.Logger, - trackID pgtype.UUID, path string, fp fingerprintResult, + trackID pgtype.UUID, path string, fp fingerprintResult, lengthSec int32, ) fingerprintOutcome { if fp.hashErr != nil { logger.Warn("fingerprint: audio stream hash failed", "path", path, "err", fp.hashErr) @@ -186,10 +210,11 @@ func storeFingerprint( // is stamped at the current version so the backfill does not retry it on // every pass. It is retried when the file changes. if err := q.UpsertTrackFingerprint(ctx, dbq.UpsertTrackFingerprintParams{ - TrackID: trackID, - AudioStreamSha256: fp.streamSHA256, - Chromaprint: fp.chromaprint, - FingerprintVersion: fingerprintVersion, + TrackID: trackID, + AudioStreamSha256: fp.streamSHA256, + Chromaprint: fp.chromaprint, + FingerprintVersion: fingerprintVersion, + ChromaprintLengthSec: lengthSec, }); err != nil { logger.Warn("fingerprint: storing fingerprint failed", "path", path, "err", err) return outcomeStoreFailed @@ -211,8 +236,8 @@ func computeAudioStreamSHA256(ctx context.Context, path string) ([]byte, error) // computeChromaprint returns the raw acoustic fingerprint of the first // lengthSec seconds of the file. -func computeChromaprint(ctx context.Context, path string, lengthSec int) ([]int32, error) { - out, err := runFingerprintTool(ctx, "fpcalc", fpcalcArgs(path, lengthSec)) +func computeChromaprint(ctx context.Context, path string, lengthSec int32) ([]int32, error) { + out, err := runFingerprintTool(ctx, "fpcalc", fpcalcArgs(path, int(lengthSec))) if err != nil { return nil, err } diff --git a/internal/library/fingerprint_backfill.go b/internal/library/fingerprint_backfill.go index 316a335b..eb19c1c8 100644 --- a/internal/library/fingerprint_backfill.go +++ b/internal/library/fingerprint_backfill.go @@ -40,10 +40,11 @@ const fingerprintBackfillTick = time.Hour // so tracks the scan adds mid-pass are not stuck behind one enormous page. const fingerprintBackfillBatch = 50 -// fingerprintBackfillConcurrency is how many files are decoded at once. Two is -// deliberately low: fpcalc and the stream hash compete with playback transcoding -// for CPU and with streaming for the mount, and a backfill that makes playback -// stutter is worse than one that takes longer. Operator-tunable in #3913. +// fingerprintBackfillConcurrency is the shipped value of the concurrency setting +// (#3913): how many files are decoded at once. Two is deliberately low: fpcalc +// and the stream hash compete with playback transcoding for CPU and with +// streaming for the mount, and a backfill that makes playback stutter is worse +// than one that takes longer. const fingerprintBackfillConcurrency = 2 // BackfillFingerprintsResult tallies one pass. @@ -68,24 +69,27 @@ func (r *BackfillFingerprintsResult) add(o fingerprintOutcome) { // FingerprintBackfillWorker fingerprints the tracks the scan never will. type FingerprintBackfillWorker struct { - pool *pgxpool.Pool - logger *slog.Logger - tick time.Duration - batch int32 - concurrency int + pool *pgxpool.Pool + logger *slog.Logger + settings *FingerprintSettingsService + tick time.Duration + batch int32 // fingerprint is a field for the same reason as Scanner.fingerprint: an // integration test pins which tracks a pass touches, not what the tools print. - fingerprint func(ctx context.Context, path string) fingerprintResult + fingerprint func(ctx context.Context, path string, opts fingerprintOptions) fingerprintResult } // NewFingerprintBackfillWorker builds a worker with the production cadence. -func NewFingerprintBackfillWorker(pool *pgxpool.Pool, logger *slog.Logger) *FingerprintBackfillWorker { +// settings is shared with the scanner and the admin API; nil runs on defaults. +func NewFingerprintBackfillWorker( + pool *pgxpool.Pool, logger *slog.Logger, settings *FingerprintSettingsService, +) *FingerprintBackfillWorker { return &FingerprintBackfillWorker{ pool: pool, logger: logger, + settings: settings, tick: fingerprintBackfillTick, batch: fingerprintBackfillBatch, - concurrency: fingerprintBackfillConcurrency, fingerprint: computeFingerprint, } } @@ -129,6 +133,12 @@ func (w *FingerprintBackfillWorker) runOnce(ctx context.Context) { // cursor is what lets a pass end: an inconclusive attempt writes no row, so a // file that keeps timing out would otherwise be listed again immediately and // retried forever within the pass. +// +// Settings are read before every batch, so a save takes effect within a batch +// rather than an hour (#3913): switching fingerprinting off ends the pass, a new +// concurrency applies to the next batch, and a new length restarts the walk from +// the top at that length, because every row written at the old one went stale +// the moment it changed. func (w *FingerprintBackfillWorker) pass(ctx context.Context) (BackfillFingerprintsResult, error) { q := dbq.New(w.pool) var ( @@ -137,15 +147,25 @@ func (w *FingerprintBackfillWorker) pass(ctx context.Context) (BackfillFingerpri ) // The all-zero uuid sorts before every real id. Valid must be true: a NULL // cursor would make "id > NULL" match nothing and every pass a silent no-op. - after := pgtype.UUID{Valid: true} + start := pgtype.UUID{Valid: true} + after := start + lengthSec := w.settings.Get().ChromaprintLengthSec for { if err := ctx.Err(); err != nil { return res, err } + cfg := w.settings.Get() + if !cfg.Enabled { + return res, nil + } + if cfg.ChromaprintLengthSec != lengthSec { + lengthSec, after = cfg.ChromaprintLengthSec, start + } rows, err := q.ListTracksNeedingFingerprint(ctx, dbq.ListTracksNeedingFingerprintParams{ - CurrentVersion: fingerprintVersion, - AfterID: after, - BatchLimit: w.batch, + CurrentVersion: fingerprintVersion, + ChromaprintLengthSec: lengthSec, + AfterID: after, + BatchLimit: w.batch, }) if err != nil { return res, fmt.Errorf("list tracks needing fingerprint: %w", err) @@ -154,7 +174,10 @@ func (w *FingerprintBackfillWorker) pass(ctx context.Context) (BackfillFingerpri return res, nil } - sem := make(chan struct{}, w.concurrency) + opts := fingerprintOptions{lengthSec: lengthSec, chromaprint: true} + // Validation keeps concurrency at one or more; the floor guards a zero + // that would block the first send for ever. + sem := make(chan struct{}, max(1, int(cfg.BackfillConcurrency))) var wg sync.WaitGroup for _, row := range rows { if ctx.Err() != nil { @@ -170,7 +193,7 @@ func (w *FingerprintBackfillWorker) pass(ctx context.Context) (BackfillFingerpri w.logger.Error("fingerprint backfill: track panicked", "path", path, "panic", r) } }() - outcome := storeFingerprint(ctx, q, w.logger, trackID, path, w.fingerprintFile(ctx, path)) + outcome := storeFingerprint(ctx, q, w.logger, trackID, path, w.fingerprintFile(ctx, path, opts), lengthSec) mu.Lock() res.add(outcome) mu.Unlock() @@ -181,16 +204,21 @@ func (w *FingerprintBackfillWorker) pass(ctx context.Context) (BackfillFingerpri } } -func (w *FingerprintBackfillWorker) fingerprintFile(ctx context.Context, path string) fingerprintResult { +func (w *FingerprintBackfillWorker) fingerprintFile(ctx context.Context, path string, opts fingerprintOptions) fingerprintResult { if w.fingerprint == nil { - return computeFingerprint(ctx, path) + return computeFingerprint(ctx, path, opts) } - return w.fingerprint(ctx, path) + return w.fingerprint(ctx, path, opts) } // FingerprintCoverage reports how much of the library carries a current // fingerprint, for the admin gauge. It lives here, beside the backfill, so the -// version it counts against is the one the backfill writes. -func FingerprintCoverage(ctx context.Context, pool *pgxpool.Pool) (dbq.GetFingerprintCoverageRow, error) { - return dbq.New(pool).GetFingerprintCoverage(ctx, fingerprintVersion) +// version and length it counts against are the ones the backfill writes. +func FingerprintCoverage( + ctx context.Context, pool *pgxpool.Pool, cfg FingerprintSettings, +) (dbq.GetFingerprintCoverageRow, error) { + return dbq.New(pool).GetFingerprintCoverage(ctx, dbq.GetFingerprintCoverageParams{ + CurrentVersion: fingerprintVersion, + ChromaprintLengthSec: cfg.ChromaprintLengthSec, + }) } diff --git a/internal/library/fingerprint_backfill_test.go b/internal/library/fingerprint_backfill_test.go index 8b5b81c9..fb3a166a 100644 --- a/internal/library/fingerprint_backfill_test.go +++ b/internal/library/fingerprint_backfill_test.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "log/slog" + "maps" "path/filepath" "sync" "testing" @@ -48,7 +49,7 @@ func TestFingerprintBackfill_Integration(t *testing.T) { } { if err := q.UpsertTrackFingerprint(ctx, dbq.UpsertTrackFingerprintParams{ TrackID: seed.track.ID, AudioStreamSha256: sum, Chromaprint: []int32{1}, - FingerprintVersion: seed.version, + FingerprintVersion: seed.version, ChromaprintLengthSec: defaultChromaprintLengthSec, }); err != nil { t.Fatalf("seed fingerprint: %v", err) } @@ -59,13 +60,24 @@ func TestFingerprintBackfill_Integration(t *testing.T) { var mu sync.Mutex calls := map[string]int{} - w := NewFingerprintBackfillWorker(pool, slog.New(slog.NewTextHandler(io.Discard, nil))) + settings, err := NewFingerprintSettingsService(ctx, pool) + if err != nil { + t.Fatalf("fingerprint settings: %v", err) + } + t.Cleanup(func() { + if _, err := settings.Set(context.Background(), DefaultFingerprintSettings); err != nil { + t.Errorf("restore fingerprint settings: %v", err) + } + }) + w := NewFingerprintBackfillWorker(pool, slog.New(slog.NewTextHandler(io.Discard, nil)), settings) // A batch of one forces the keyset cursor across several queries in a pass. w.batch = 1 - w.fingerprint = func(_ context.Context, path string) fingerprintResult { + lengths := map[int32]int{} + w.fingerprint = func(_ context.Context, path string, opts fingerprintOptions) fingerprintResult { name := filepath.Base(path) mu.Lock() calls[name]++ + lengths[opts.lengthSec]++ mu.Unlock() switch name { case "stall.mp3": @@ -126,7 +138,7 @@ func TestFingerprintBackfill_Integration(t *testing.T) { } // 4. The gauge counts what the passes wrote, and its buckets add up. - cov, err := FingerprintCoverage(ctx, pool) + cov, err := FingerprintCoverage(ctx, pool, settings.Get()) if err != nil { t.Fatalf("coverage: %v", err) } @@ -138,6 +150,64 @@ func TestFingerprintBackfill_Integration(t *testing.T) { if cov.Fingerprinted+cov.Rejected+cov.Pending != cov.Total { t.Errorf("coverage buckets %+v do not sum to the total", cov) } + + // 5. Changing the length (#3913) makes every stored row stale at once — the + // gauge shows the whole library pending before the backfill has touched a + // file — and the next pass re-derives each at the new length. The failure + // this prevents is invisible from the UI: prints at two lengths that silently + // never match. + shorter := DefaultFingerprintSettings + shorter.ChromaprintLengthSec = 60 + if _, err := settings.Set(ctx, shorter); err != nil { + t.Fatalf("change length: %v", err) + } + cov, err = FingerprintCoverage(ctx, pool, settings.Get()) + if err != nil { + t.Fatalf("coverage after length change: %v", err) + } + if cov.Total != 5 || cov.Pending != 5 { + t.Fatalf("coverage after length change = %+v, want all 5 tracks pending", cov) + } + mu.Lock() + clear(lengths) + mu.Unlock() + res, err = w.pass(ctx) + if err != nil { + t.Fatalf("new-length pass: %v", err) + } + if res.Processed != 5 { + t.Fatalf("new-length pass = %+v, want all 5 present tracks re-derived", res) + } + mu.Lock() + asked := maps.Clone(lengths) + mu.Unlock() + if len(asked) != 1 || asked[60] != 5 { + t.Fatalf("new-length pass asked for lengths %v, want 60s for all 5", asked) + } + var atOldLength int + if err := pool.QueryRow(ctx, + "SELECT count(*) FROM track_fingerprints WHERE chromaprint_length_sec <> 60").Scan(&atOldLength); err != nil { + t.Fatalf("count old-length rows: %v", err) + } + if atOldLength != 0 { + t.Fatalf("%d fingerprints are still at the old length after a complete pass", atOldLength) + } + + // 6. Switched off, the backfill does nothing, even with work waiting. + off := DefaultFingerprintSettings + off.Enabled = false + if _, err := settings.Set(ctx, off); err != nil { + t.Fatalf("switch fingerprinting off: %v", err) + } + addTrack("later") + res, err = w.pass(ctx) + if err != nil { + t.Fatalf("pass with fingerprinting off: %v", err) + } + if res.Processed != 0 || callCount("later.mp3") != 0 { + t.Fatalf("pass with fingerprinting off = %+v (later.mp3 tried %d times), want nothing done", + res, callCount("later.mp3")) + } } func TestBackfillFingerprintsResult_Add(t *testing.T) { diff --git a/internal/library/fingerprint_scan_test.go b/internal/library/fingerprint_scan_test.go index b2edb800..1e4c1d0a 100644 --- a/internal/library/fingerprint_scan_test.go +++ b/internal/library/fingerprint_scan_test.go @@ -60,9 +60,24 @@ func TestScanner_FingerprintsOnlyNewOrChangedBytes_Integration(t *testing.T) { result := fingerprintResult{streamSHA256: sum, chromaprint: chroma} calls := map[string]int{} - scanner := New(pool, logger, []string{root}) - scanner.fingerprint = func(_ context.Context, path string) fingerprintResult { + settings, err := NewFingerprintSettingsService(ctx, pool) + if err != nil { + t.Fatalf("fingerprint settings: %v", err) + } + if _, err := settings.Set(ctx, DefaultFingerprintSettings); err != nil { + t.Fatalf("reset fingerprint settings: %v", err) + } + t.Cleanup(func() { + if _, err := settings.Set(context.Background(), DefaultFingerprintSettings); err != nil { + t.Errorf("restore fingerprint settings: %v", err) + } + }) + + var lastOpts fingerprintOptions + scanner := New(pool, logger, []string{root}, settings) + scanner.fingerprint = func(_ context.Context, path string, opts fingerprintOptions) fingerprintResult { calls[path]++ + lastOpts = opts return result } @@ -78,14 +93,15 @@ func TestScanner_FingerprintsOnlyNewOrChangedBytes_Integration(t *testing.T) { sha []byte chroma []int32 version int16 + length int32 } stored := func(path string) (row, bool) { t.Helper() var r row err := pool.QueryRow(ctx, ` - SELECT f.audio_stream_sha256, f.chromaprint, f.fingerprint_version + SELECT f.audio_stream_sha256, f.chromaprint, f.fingerprint_version, f.chromaprint_length_sec FROM track_fingerprints f JOIN tracks t ON t.id = f.track_id - WHERE t.file_path = $1`, path).Scan(&r.sha, &r.chroma, &r.version) + WHERE t.file_path = $1`, path).Scan(&r.sha, &r.chroma, &r.version, &r.length) if errors.Is(err, pgx.ErrNoRows) { return row{}, false } @@ -113,8 +129,13 @@ func TestScanner_FingerprintsOnlyNewOrChangedBytes_Integration(t *testing.T) { if !ok { t.Fatal("first scan stored no fingerprint") } - if !bytes.Equal(got.sha, sum) || !slices.Equal(got.chroma, chroma) || got.version != fingerprintVersion { - t.Fatalf("stored %+v, want sha %x chromaprint %v version %d", got, sum, chroma, fingerprintVersion) + if !bytes.Equal(got.sha, sum) || !slices.Equal(got.chroma, chroma) || got.version != fingerprintVersion || + got.length != defaultChromaprintLengthSec { + t.Fatalf("stored %+v, want sha %x chromaprint %v version %d length %d", + got, sum, chroma, fingerprintVersion, defaultChromaprintLengthSec) + } + if !lastOpts.chromaprint || lastOpts.lengthSec != defaultChromaprintLengthSec { + t.Fatalf("first scan asked for %+v, want a chromaprint at the default length", lastOpts) } // 2. A tag-repair pass re-reads every unchanged file and must not @@ -171,4 +192,39 @@ func TestScanner_FingerprintsOnlyNewOrChangedBytes_Integration(t *testing.T) { if got.sha != nil || got.chroma != nil || got.version != fingerprintVersion { t.Fatalf("rejected file stored %+v, want both halves NULL at version %d", got, fingerprintVersion) } + + // 6. With fingerprinting off (#3913) the scan decodes nothing. It still asks + // for the stream hash — a demux, and what recognises a moved file — but stores + // no row, and a changed file's old row goes: it describes bytes that are gone. + result = fingerprintResult{streamSHA256: sum, chromaprint: chroma} + off := DefaultFingerprintSettings + off.Enabled = false + if _, err := settings.Set(ctx, off); err != nil { + t.Fatalf("switch fingerprinting off: %v", err) + } + touch(a, 4*time.Hour) + scan("fingerprinting-off scan") + if calls[a] != 5 || lastOpts.chromaprint { + t.Fatalf("fingerprinting-off scan: calls = %v, last options %+v; want a fifth call asking for no chromaprint", + calls, lastOpts) + } + if _, ok := stored(a); ok { + t.Fatal("with fingerprinting off, a changed file kept the previous bytes' fingerprint") + } + if _, ok := stored(b); !ok { + t.Fatal("with fingerprinting off, an unchanged file lost its fingerprint") + } + + // 7. The length setting reaches the scan, and is stored with the row. + longer := DefaultFingerprintSettings + longer.ChromaprintLengthSec = 90 + if _, err := settings.Set(ctx, longer); err != nil { + t.Fatalf("change length: %v", err) + } + touch(a, 5*time.Hour) + scan("new-length scan") + got, ok = stored(a) + if !ok || got.length != 90 || lastOpts.lengthSec != 90 || !lastOpts.chromaprint { + t.Fatalf("new-length scan stored %+v (present %v) after asking for %+v; want a chromaprint at 90s", got, ok, lastOpts) + } } diff --git a/internal/library/fingerprint_settings.go b/internal/library/fingerprint_settings.go new file mode 100644 index 00000000..fa971b80 --- /dev/null +++ b/internal/library/fingerprint_settings.go @@ -0,0 +1,163 @@ +package library + +import ( + "context" + "errors" + "fmt" + "sync" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Fingerprinting settings (M400 #3913). +// +// Rule 25: what an operator might tune lives in the database and changes without +// a restart. One service instance is shared by the scanner, the backfill, the +// duplicate sweep and the admin API, so a save reaches all of them at once. +// +// The length is the dangerous knob: chromaprints taken at two lengths cannot be +// compared. Rather than bumping fingerprintVersion when it changes, every row +// records the length it was taken at (migration 0061) and every reader filters +// on the current one. A row at another length is stale to the backfill, pending +// to the gauge and invisible to the sweep, which is the effect a version bump +// would have, with one difference worth having: setting the length back makes +// rows not yet re-derived current again, instead of redoing the library twice. + +// Bounds for each setting, mirrored by migration 0061's CHECKs. +const ( + // Below 30s a print has too few items to align at the matcher's larger + // offsets and still overlap by minOverlapItems. Above 600s each print passes + // 20 KB and a candidate page of them grows past what one sweep should hold. + minChromaprintLengthSec = 30 + maxChromaprintLengthSec = 600 + // Unrelated audio sits near 0.5; a ceiling of 0.35 keeps a loosened + // threshold well clear of proposing noise. + minAcousticMaxBitErrorRate = 0.01 + maxAcousticMaxBitErrorRate = 0.35 + minBackfillConcurrency = 1 + maxBackfillConcurrency = 8 + minSweepIntervalHours = 1 + maxSweepIntervalHours = 168 + + defaultSweepIntervalHours = 1 +) + +// FingerprintSettings mirrors the fingerprint_settings row. +type FingerprintSettings struct { + // Enabled off stops every decode: the scan takes only the stream hash (a + // demux, and what recognises a moved file) and stores nothing, and the + // backfill idles. + Enabled bool + ChromaprintLengthSec int32 + AcousticMaxBitErrorRate float64 + BackfillConcurrency int32 + SweepIntervalHours int32 + // UpdatedAt is when the settings were last saved. Set by the database; + // ignored by Set. + UpdatedAt time.Time +} + +// DefaultFingerprintSettings mirrors migration 0061's column defaults, so a +// database that cannot be read still fingerprints the way a fresh install does. +var DefaultFingerprintSettings = FingerprintSettings{ + Enabled: true, + ChromaprintLengthSec: defaultChromaprintLengthSec, + AcousticMaxBitErrorRate: defaultAcousticMaxBitErrorRate, + BackfillConcurrency: fingerprintBackfillConcurrency, + SweepIntervalHours: defaultSweepIntervalHours, +} + +// ErrFingerprintSettingOutOfRange is returned by Set for a value migration +// 0061's CHECKs would reject, so the API answers 400 naming the field rather +// than surfacing a constraint violation. +var ErrFingerprintSettingOutOfRange = errors.New("fingerprint setting out of range") + +// FingerprintSettingsService caches the settings and owns their persistence. +// Cached because the scanner reads them for every file it fingerprints. +type FingerprintSettingsService struct { + pool *pgxpool.Pool + + mu sync.RWMutex + cur FingerprintSettings +} + +// NewFingerprintSettingsService loads once and caches. It always returns a +// usable service, holding the defaults when the load fails; the error says so. +func NewFingerprintSettingsService(ctx context.Context, pool *pgxpool.Pool) (*FingerprintSettingsService, error) { + s := &FingerprintSettingsService{pool: pool, cur: DefaultFingerprintSettings} + row, err := dbq.New(pool).GetFingerprintSettings(ctx) + if err != nil { + return s, fmt.Errorf("fingerprint settings: load: %w", err) + } + s.cur = fingerprintSettingsFromRow(row) + return s, nil +} + +// Get returns the cached settings. A nil service answers with the defaults, so +// a Scanner or worker built without one fingerprints as a fresh install would. +func (s *FingerprintSettingsService) Get() FingerprintSettings { + if s == nil { + return DefaultFingerprintSettings + } + s.mu.RLock() + defer s.mu.RUnlock() + return s.cur +} + +// Set validates, persists and re-caches. +func (s *FingerprintSettingsService) Set(ctx context.Context, in FingerprintSettings) (FingerprintSettings, error) { + if err := validateFingerprintSettings(in); err != nil { + return FingerprintSettings{}, err + } + if s == nil { + return FingerprintSettings{}, errors.New("fingerprint settings: no settings service") + } + row, err := dbq.New(s.pool).UpdateFingerprintSettings(ctx, dbq.UpdateFingerprintSettingsParams{ + Enabled: in.Enabled, + ChromaprintLengthSec: in.ChromaprintLengthSec, + AcousticMaxBitErrorRate: in.AcousticMaxBitErrorRate, + BackfillConcurrency: in.BackfillConcurrency, + SweepIntervalHours: in.SweepIntervalHours, + }) + if err != nil { + return FingerprintSettings{}, fmt.Errorf("fingerprint settings: save: %w", err) + } + out := fingerprintSettingsFromRow(row) + s.mu.Lock() + s.cur = out + s.mu.Unlock() + return out, nil +} + +func validateFingerprintSettings(in FingerprintSettings) error { + switch { + case in.ChromaprintLengthSec < minChromaprintLengthSec || in.ChromaprintLengthSec > maxChromaprintLengthSec: + return fmt.Errorf("%w: chromaprint_length_sec must be %d-%d", + ErrFingerprintSettingOutOfRange, minChromaprintLengthSec, maxChromaprintLengthSec) + // Written as a negated range so NaN, which fails every comparison, is refused. + case !(in.AcousticMaxBitErrorRate >= minAcousticMaxBitErrorRate && in.AcousticMaxBitErrorRate <= maxAcousticMaxBitErrorRate): + return fmt.Errorf("%w: acoustic_max_bit_error_rate must be %.2f-%.2f", + ErrFingerprintSettingOutOfRange, minAcousticMaxBitErrorRate, maxAcousticMaxBitErrorRate) + case in.BackfillConcurrency < minBackfillConcurrency || in.BackfillConcurrency > maxBackfillConcurrency: + return fmt.Errorf("%w: backfill_concurrency must be %d-%d", + ErrFingerprintSettingOutOfRange, minBackfillConcurrency, maxBackfillConcurrency) + case in.SweepIntervalHours < minSweepIntervalHours || in.SweepIntervalHours > maxSweepIntervalHours: + return fmt.Errorf("%w: sweep_interval_hours must be %d-%d", + ErrFingerprintSettingOutOfRange, minSweepIntervalHours, maxSweepIntervalHours) + } + return nil +} + +func fingerprintSettingsFromRow(row dbq.FingerprintSetting) FingerprintSettings { + return FingerprintSettings{ + Enabled: row.Enabled, + ChromaprintLengthSec: row.ChromaprintLengthSec, + AcousticMaxBitErrorRate: row.AcousticMaxBitErrorRate, + BackfillConcurrency: row.BackfillConcurrency, + SweepIntervalHours: row.SweepIntervalHours, + UpdatedAt: row.UpdatedAt.Time, + } +} diff --git a/internal/library/fingerprint_settings_test.go b/internal/library/fingerprint_settings_test.go new file mode 100644 index 00000000..95d5837b --- /dev/null +++ b/internal/library/fingerprint_settings_test.go @@ -0,0 +1,146 @@ +package library + +import ( + "context" + "errors" + "math" + "testing" + "time" +) + +func TestValidateFingerprintSettings(t *testing.T) { + with := func(edit func(*FingerprintSettings)) FingerprintSettings { + s := DefaultFingerprintSettings + edit(&s) + return s + } + valid := map[string]FingerprintSettings{ + "defaults": DefaultFingerprintSettings, + "shortest length": with(func(s *FingerprintSettings) { s.ChromaprintLengthSec = minChromaprintLengthSec }), + "longest length": with(func(s *FingerprintSettings) { s.ChromaprintLengthSec = maxChromaprintLengthSec }), + "strictest match": with(func(s *FingerprintSettings) { s.AcousticMaxBitErrorRate = minAcousticMaxBitErrorRate }), + "loosest match": with(func(s *FingerprintSettings) { s.AcousticMaxBitErrorRate = maxAcousticMaxBitErrorRate }), + "fewest at once": with(func(s *FingerprintSettings) { s.BackfillConcurrency = minBackfillConcurrency }), + "most at once": with(func(s *FingerprintSettings) { s.BackfillConcurrency = maxBackfillConcurrency }), + "shortest interval": with(func(s *FingerprintSettings) { s.SweepIntervalHours = minSweepIntervalHours }), + "longest interval": with(func(s *FingerprintSettings) { s.SweepIntervalHours = maxSweepIntervalHours }), + "switched off": with(func(s *FingerprintSettings) { s.Enabled = false }), + } + for name, s := range valid { + if err := validateFingerprintSettings(s); err != nil { + t.Errorf("%s: rejected: %v", name, err) + } + } + invalid := map[string]FingerprintSettings{ + "length too short": with(func(s *FingerprintSettings) { s.ChromaprintLengthSec = minChromaprintLengthSec - 1 }), + "length too long": with(func(s *FingerprintSettings) { s.ChromaprintLengthSec = maxChromaprintLengthSec + 1 }), + "match too strict": with(func(s *FingerprintSettings) { s.AcousticMaxBitErrorRate = minAcousticMaxBitErrorRate - 0.001 }), + "match too loose": with(func(s *FingerprintSettings) { s.AcousticMaxBitErrorRate = maxAcousticMaxBitErrorRate + 0.001 }), + "match not a number": with(func(s *FingerprintSettings) { s.AcousticMaxBitErrorRate = math.NaN() }), + "none at once": with(func(s *FingerprintSettings) { s.BackfillConcurrency = 0 }), + "too many at once": with(func(s *FingerprintSettings) { s.BackfillConcurrency = maxBackfillConcurrency + 1 }), + "no interval": with(func(s *FingerprintSettings) { s.SweepIntervalHours = 0 }), + "interval too long": with(func(s *FingerprintSettings) { s.SweepIntervalHours = maxSweepIntervalHours + 1 }), + } + for name, s := range invalid { + if err := validateFingerprintSettings(s); !errors.Is(err, ErrFingerprintSettingOutOfRange) { + t.Errorf("%s: err = %v, want ErrFingerprintSettingOutOfRange", name, err) + } + } +} + +func TestFingerprintSettingsService_NilServesDefaults(t *testing.T) { + var s *FingerprintSettingsService + if got := s.Get(); got != DefaultFingerprintSettings { + t.Fatalf("nil service Get = %+v, want the defaults", got) + } + // Validation still runs first, so a bad value is named rather than hidden + // behind the missing service. + bad := DefaultFingerprintSettings + bad.BackfillConcurrency = 0 + if _, err := s.Set(context.Background(), bad); !errors.Is(err, ErrFingerprintSettingOutOfRange) { + t.Fatalf("nil service Set of a bad value: err = %v, want ErrFingerprintSettingOutOfRange", err) + } +} + +func TestFingerprintSettingsService_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + reload := func(step string) FingerprintSettings { + t.Helper() + fresh, err := NewFingerprintSettingsService(ctx, pool) + if err != nil { + t.Fatalf("%s: load: %v", step, err) + } + return fresh.Get() + } + withoutTime := func(s FingerprintSettings) FingerprintSettings { + s.UpdatedAt = time.Time{} + return s + } + + svc, err := NewFingerprintSettingsService(ctx, pool) + if err != nil { + t.Fatalf("load: %v", err) + } + // ResetDB puts every column back to its migration default, so this pins the + // Go defaults to migration 0061's. Were they to drift, a database that could + // not be read would fingerprint differently from one that could. + loaded := svc.Get() + if loaded.UpdatedAt.IsZero() { + t.Fatal("loaded settings carry no updated_at") + } + if withoutTime(loaded) != DefaultFingerprintSettings { + t.Fatalf("stored defaults = %+v, want the Go defaults %+v", withoutTime(loaded), DefaultFingerprintSettings) + } + + // Every bound the service accepts, the table accepts too. A CHECK tighter + // than validate would turn a value the card allows into a 500. + lowest := FingerprintSettings{ + Enabled: false, + ChromaprintLengthSec: minChromaprintLengthSec, + AcousticMaxBitErrorRate: minAcousticMaxBitErrorRate, + BackfillConcurrency: minBackfillConcurrency, + SweepIntervalHours: minSweepIntervalHours, + } + highest := FingerprintSettings{ + Enabled: true, + ChromaprintLengthSec: maxChromaprintLengthSec, + AcousticMaxBitErrorRate: maxAcousticMaxBitErrorRate, + BackfillConcurrency: maxBackfillConcurrency, + SweepIntervalHours: maxSweepIntervalHours, + } + for _, step := range []struct { + name string + want FingerprintSettings + }{{"lowest", lowest}, {"highest", highest}} { + saved, err := svc.Set(ctx, step.want) + if err != nil { + t.Fatalf("%s: save: %v", step.name, err) + } + // A save must move updated_at forward: it is what makes a sweep due. + if !saved.UpdatedAt.After(loaded.UpdatedAt) { + t.Fatalf("%s: updated_at %v did not move past %v", step.name, saved.UpdatedAt, loaded.UpdatedAt) + } + if withoutTime(saved) != step.want || withoutTime(svc.Get()) != step.want { + t.Fatalf("%s: saved %+v, cached %+v, want %+v", step.name, saved, svc.Get(), step.want) + } + if got := withoutTime(reload(step.name)); got != step.want { + t.Fatalf("%s: table holds %+v, want %+v", step.name, got, step.want) + } + } + + // An out-of-range save changes nothing, in the cache or the table. + before := svc.Get() + bad := highest + bad.ChromaprintLengthSec = maxChromaprintLengthSec + 1 + if _, err := svc.Set(ctx, bad); !errors.Is(err, ErrFingerprintSettingOutOfRange) { + t.Fatalf("out-of-range save: err = %v, want ErrFingerprintSettingOutOfRange", err) + } + if svc.Get() != before { + t.Fatalf("out-of-range save changed the cache to %+v", svc.Get()) + } + if got := reload("after out-of-range save"); got != before { + t.Fatalf("out-of-range save changed the table to %+v", got) + } +} diff --git a/internal/library/moved.go b/internal/library/moved.go index aa049e8f..eb453ccd 100644 --- a/internal/library/moved.go +++ b/internal/library/moved.go @@ -41,7 +41,7 @@ import ( // match/ambiguity logic can be tested against a fake. type trackAdopter interface { FindMissingTrackByMbid(ctx context.Context, mbid string) ([]dbq.FindMissingTrackByMbidRow, error) - FindMissingTrackByFingerprint(ctx context.Context, arg dbq.FindMissingTrackByFingerprintParams) ([]dbq.FindMissingTrackByFingerprintRow, error) + FindMissingTrackByAudioHash(ctx context.Context, audioStreamSha256 []byte) ([]dbq.FindMissingTrackByAudioHashRow, error) AdoptTrackPath(ctx context.Context, arg dbq.AdoptTrackPathParams) (int64, error) } @@ -53,10 +53,10 @@ type trackAdopter interface { // pre-#2528 behaviour. func (s *Scanner) adoptMovedTrack( ctx context.Context, q trackAdopter, newPath string, - fileSize int64, durationMs int32, recordingMBID string, + audioHash []byte, recordingMBID string, ) bool { // MBID first. It identifies the recording rather than the bytes, so it - // survives a re-encode that the fingerprint cannot. + // survives a re-encode that the audio hash cannot. if recordingMBID != "" { rows, err := q.FindMissingTrackByMbid(ctx, recordingMBID) if err != nil { @@ -67,19 +67,17 @@ func (s *Scanner) adoptMovedTrack( } } - // Fingerprint fallback for untagged files. Both components must be real: - // duration_ms is 0 when ffprobe failed, and matching 0 against 0 would pair - // up unrelated broken files. - if fileSize > 0 && durationMs > 0 { - rows, err := q.FindMissingTrackByFingerprint(ctx, dbq.FindMissingTrackByFingerprintParams{ - FileSize: fileSize, - DurationMs: durationMs, - }) + // Audio-hash fallback for untagged files (#3914): the encoded audio itself, + // which a rename, a move or a retag leaves unchanged. Absent when the hash + // could not be taken, and then nothing is matched — an empty hash must never + // be looked up, or every unhashable file would pair with every other. + if len(audioHash) > 0 { + rows, err := q.FindMissingTrackByAudioHash(ctx, audioHash) if err != nil { - s.logger.Warn("library scan: move lookup by fingerprint failed", + s.logger.Warn("library scan: move lookup by audio hash failed", "path", newPath, "err", err) - } else if c, ok := s.uniqueMatch(rowsFromFingerprint(rows), newPath, "fingerprint"); ok { - return s.adopt(ctx, q, c, newPath, "fingerprint") + } else if c, ok := s.uniqueMatch(rowsFromAudioHash(rows), newPath, "audio_hash"); ok { + return s.adopt(ctx, q, c, newPath, "audio_hash") } } @@ -100,7 +98,7 @@ func rowsFromMbid(rows []dbq.FindMissingTrackByMbidRow) []candidate { return out } -func rowsFromFingerprint(rows []dbq.FindMissingTrackByFingerprintRow) []candidate { +func rowsFromAudioHash(rows []dbq.FindMissingTrackByAudioHashRow) []candidate { out := make([]candidate, 0, len(rows)) for _, r := range rows { out = append(out, candidate{id: r.ID, filePath: r.FilePath}) diff --git a/internal/library/moved_test.go b/internal/library/moved_test.go index f3536db9..561dfff2 100644 --- a/internal/library/moved_test.go +++ b/internal/library/moved_test.go @@ -1,6 +1,7 @@ package library import ( + "bytes" "context" "errors" "testing" @@ -10,18 +11,21 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) +// fakeAdopter answers the audio-hash lookup only for the hash it holds, the way +// the real query does. A fake that returned its rows for any hash could not tell +// adoption by identity apart from adoption by coincidence. type fakeAdopter struct { - byMbid []dbq.FindMissingTrackByMbidRow - byFingerprint []dbq.FindMissingTrackByFingerprintRow + byMbid []dbq.FindMissingTrackByMbidRow + hash []byte + byHash []dbq.FindMissingTrackByAudioHashRow + mbidErr error + hashErr error + adoptErr error + adoptRows int64 - mbidErr error - fingerprintErr error - adoptErr error - adoptRows int64 - - mbidQueried []string - fingerprintQueried []dbq.FindMissingTrackByFingerprintParams - adopted []dbq.AdoptTrackPathParams + mbidQueried []string + hashQueried [][]byte + adopted []dbq.AdoptTrackPathParams } func (f *fakeAdopter) FindMissingTrackByMbid(_ context.Context, mbid string) ([]dbq.FindMissingTrackByMbidRow, error) { @@ -29,11 +33,17 @@ func (f *fakeAdopter) FindMissingTrackByMbid(_ context.Context, mbid string) ([] return f.byMbid, f.mbidErr } -func (f *fakeAdopter) FindMissingTrackByFingerprint( - _ context.Context, arg dbq.FindMissingTrackByFingerprintParams, -) ([]dbq.FindMissingTrackByFingerprintRow, error) { - f.fingerprintQueried = append(f.fingerprintQueried, arg) - return f.byFingerprint, f.fingerprintErr +func (f *fakeAdopter) FindMissingTrackByAudioHash( + _ context.Context, audioStreamSha256 []byte, +) ([]dbq.FindMissingTrackByAudioHashRow, error) { + f.hashQueried = append(f.hashQueried, audioStreamSha256) + if f.hashErr != nil { + return nil, f.hashErr + } + if !bytes.Equal(audioStreamSha256, f.hash) { + return nil, nil + } + return f.byHash, nil } func (f *fakeAdopter) AdoptTrackPath(_ context.Context, arg dbq.AdoptTrackPathParams) (int64, error) { @@ -51,10 +61,12 @@ func mbidRow(n byte, path string) dbq.FindMissingTrackByMbidRow { return dbq.FindMissingTrackByMbidRow{ID: testUUID(n), FilePath: path} } -func fpRow(n byte, path string) dbq.FindMissingTrackByFingerprintRow { - return dbq.FindMissingTrackByFingerprintRow{ID: testUUID(n), FilePath: path} +func hashRow(n byte, path string) dbq.FindMissingTrackByAudioHashRow { + return dbq.FindMissingTrackByAudioHashRow{ID: testUUID(n), FilePath: path} } +func audioHash(b byte) []byte { return bytes.Repeat([]byte{b}, 32) } + const ( oldPath = "/music/Linkin Park/Minutes to Midnight/02 - Bleed It Out.mp3" newPath = "/music/Linkin Park/Minutes to Midnight/04 - Bleed It Out.mp3" @@ -64,47 +76,53 @@ func TestAdoptMovedTrack_MatchesByMbid(t *testing.T) { s := testScanner(t) q := &fakeAdopter{byMbid: []dbq.FindMissingTrackByMbidRow{mbidRow(7, oldPath)}, adoptRows: 1} - if !s.adoptMovedTrack(context.Background(), q, newPath, 5_000_000, 200_000, "rec-mbid") { + if !s.adoptMovedTrack(context.Background(), q, newPath, audioHash(1), "rec-mbid") { t.Fatal("expected the moved track to be adopted") } - if len(q.adopted) != 1 { - t.Fatalf("adopted %d rows, want 1", len(q.adopted)) - } - if q.adopted[0].ID != testUUID(7) { - t.Errorf("adopted the wrong row: %v", q.adopted[0].ID) - } - if q.adopted[0].FilePath != newPath { - t.Errorf("adopted FilePath = %q, want %q", q.adopted[0].FilePath, newPath) + if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(7) || q.adopted[0].FilePath != newPath { + t.Fatalf("adopted = %+v, want row 7 at %q", q.adopted, newPath) } // MBID matched, so the weaker signal should not have been consulted. - if len(q.fingerprintQueried) != 0 { - t.Errorf("queried the fingerprint despite an MBID match") + if len(q.hashQueried) != 0 { + t.Errorf("queried the audio hash despite an MBID match") } } -func TestAdoptMovedTrack_FallsBackToFingerprint(t *testing.T) { +func TestAdoptMovedTrack_FallsBackToAudioHash(t *testing.T) { s := testScanner(t) - q := &fakeAdopter{byFingerprint: []dbq.FindMissingTrackByFingerprintRow{fpRow(3, oldPath)}, adoptRows: 1} + q := &fakeAdopter{hash: audioHash(3), byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(3, oldPath)}, adoptRows: 1} // No MBID: an untagged file, which is exactly what the fallback is for. - if !s.adoptMovedTrack(context.Background(), q, newPath, 4_200_000, 187_000, "") { - t.Fatal("expected adoption via fingerprint") + if !s.adoptMovedTrack(context.Background(), q, newPath, audioHash(3), "") { + t.Fatal("expected adoption via the audio hash") } if len(q.mbidQueried) != 0 { t.Errorf("queried by MBID with no MBID available") } - if len(q.fingerprintQueried) != 1 { - t.Fatalf("fingerprint queried %d times, want 1", len(q.fingerprintQueried)) - } - got := q.fingerprintQueried[0] - if got.FileSize != 4_200_000 || got.DurationMs != 187_000 { - t.Errorf("fingerprint = %+v, want size 4200000 duration 187000", got) + if len(q.hashQueried) != 1 || !bytes.Equal(q.hashQueried[0], audioHash(3)) { + t.Fatalf("hash queried = %x, want exactly the file's hash", q.hashQueried) } if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(3) { t.Errorf("adopted = %+v, want row 3", q.adopted) } } +// The case the old (file_size, duration_ms) pair got wrong: an unrelated file +// that happens to share a size and a duration with a missing track. Size and +// duration are no longer inputs at all; only the audio itself can match, and a +// different recording has a different hash. +func TestAdoptMovedTrack_DifferentAudioIsNotAdopted(t *testing.T) { + s := testScanner(t) + q := &fakeAdopter{hash: audioHash(0xAA), byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(1, oldPath)}, adoptRows: 1} + + if s.adoptMovedTrack(context.Background(), q, newPath, audioHash(0xBB), "") { + t.Fatal("adopted a missing track whose audio differs") + } + if len(q.adopted) != 0 { + t.Errorf("adopted = %+v, want none", q.adopted) + } +} + // Two missing rows carrying the same recording MBID means real duplicates. // Adopting one arbitrarily would attach this file's future history to a coin // flip, so it must insert fresh instead. @@ -115,7 +133,7 @@ func TestAdoptMovedTrack_RefusesAmbiguousMbidMatch(t *testing.T) { mbidRow(2, "/music/b.mp3"), }, adoptRows: 1} - if s.adoptMovedTrack(context.Background(), q, newPath, 0, 0, "rec-mbid") { + if s.adoptMovedTrack(context.Background(), q, newPath, nil, "rec-mbid") { t.Fatal("expected refusal on an ambiguous MBID match") } if len(q.adopted) != 0 { @@ -123,66 +141,57 @@ func TestAdoptMovedTrack_RefusesAmbiguousMbidMatch(t *testing.T) { } } -// An ambiguous MBID may still be resolvable by the fingerprint, which is a +// An ambiguous MBID may still be resolvable by the audio hash, which is a // narrower signal — so falling through is allowed to succeed. -func TestAdoptMovedTrack_AmbiguousMbidFallsThroughToFingerprint(t *testing.T) { +func TestAdoptMovedTrack_AmbiguousMbidFallsThroughToAudioHash(t *testing.T) { s := testScanner(t) q := &fakeAdopter{ byMbid: []dbq.FindMissingTrackByMbidRow{ mbidRow(1, "/music/a.mp3"), mbidRow(2, "/music/b.mp3"), }, - byFingerprint: []dbq.FindMissingTrackByFingerprintRow{fpRow(2, "/music/b.mp3")}, - adoptRows: 1, + hash: audioHash(2), + byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(2, "/music/b.mp3")}, + adoptRows: 1, } - if !s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "rec-mbid") { - t.Fatal("expected the fingerprint to disambiguate") + if !s.adoptMovedTrack(context.Background(), q, newPath, audioHash(2), "rec-mbid") { + t.Fatal("expected the audio hash to disambiguate") } if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(2) { t.Errorf("adopted = %+v, want row 2", q.adopted) } } -func TestAdoptMovedTrack_RefusesAmbiguousFingerprintMatch(t *testing.T) { +// Two missing tracks with identical audio are duplicates of each other; a new +// file matching both cannot be assigned to either. +func TestAdoptMovedTrack_RefusesAmbiguousAudioHashMatch(t *testing.T) { s := testScanner(t) - q := &fakeAdopter{byFingerprint: []dbq.FindMissingTrackByFingerprintRow{ - fpRow(1, "/music/a.mp3"), - fpRow(2, "/music/b.mp3"), + q := &fakeAdopter{hash: audioHash(5), byHash: []dbq.FindMissingTrackByAudioHashRow{ + hashRow(1, "/music/a.mp3"), + hashRow(2, "/music/b.mp3"), }, adoptRows: 1} - if s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "") { - t.Fatal("expected refusal on an ambiguous fingerprint match") + if s.adoptMovedTrack(context.Background(), q, newPath, audioHash(5), "") { + t.Fatal("expected refusal on an ambiguous audio-hash match") } if len(q.adopted) != 0 { t.Errorf("adopted despite ambiguity: %+v", q.adopted) } } -// duration_ms is 0 when ffprobe failed. Matching 0 against 0 would pair up -// unrelated broken files, so the fingerprint must not be attempted. -func TestAdoptMovedTrack_SkipsFingerprintWithoutRealValues(t *testing.T) { - tests := []struct { - name string - size int64 - duration int32 - }{ - {"no duration", 1000, 0}, - {"no size", 0, 2000}, - {"neither", 0, 0}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { +// No hash means it could not be taken. An empty hash must never be looked up: +// every unhashable file would pair with every other. +func TestAdoptMovedTrack_SkipsAudioHashWhenAbsent(t *testing.T) { + for name, hash := range map[string][]byte{"nil": nil, "empty": {}} { + t.Run(name, func(t *testing.T) { s := testScanner(t) - q := &fakeAdopter{ - byFingerprint: []dbq.FindMissingTrackByFingerprintRow{fpRow(1, oldPath)}, - adoptRows: 1, + q := &fakeAdopter{hash: hash, byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(1, oldPath)}, adoptRows: 1} + if s.adoptMovedTrack(context.Background(), q, newPath, hash, "") { + t.Error("adopted without an audio hash") } - if s.adoptMovedTrack(context.Background(), q, newPath, tc.size, tc.duration, "") { - t.Error("adopted on an unusable fingerprint") - } - if len(q.fingerprintQueried) != 0 { - t.Error("queried the fingerprint with unusable values") + if len(q.hashQueried) != 0 { + t.Error("looked up an absent audio hash") } }) } @@ -192,7 +201,7 @@ func TestAdoptMovedTrack_NoCandidates(t *testing.T) { s := testScanner(t) q := &fakeAdopter{adoptRows: 1} - if s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "rec-mbid") { + if s.adoptMovedTrack(context.Background(), q, newPath, audioHash(9), "rec-mbid") { t.Fatal("expected no adoption when nothing matches") } if len(q.adopted) != 0 { @@ -209,7 +218,7 @@ func TestAdoptMovedTrack_LostRaceReportsNotAdopted(t *testing.T) { adoptRows: 0, } - if s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "rec-mbid") { + if s.adoptMovedTrack(context.Background(), q, newPath, audioHash(5), "rec-mbid") { t.Fatal("expected not-adopted when the update matched no rows") } } @@ -223,7 +232,7 @@ func TestAdoptMovedTrack_ToleratesQueryErrors(t *testing.T) { q *fakeAdopter }{ {"mbid lookup fails", &fakeAdopter{mbidErr: sentinel}}, - {"fingerprint lookup fails", &fakeAdopter{fingerprintErr: sentinel}}, + {"audio hash lookup fails", &fakeAdopter{hashErr: sentinel}}, {"adopt fails", &fakeAdopter{ byMbid: []dbq.FindMissingTrackByMbidRow{mbidRow(1, oldPath)}, adoptErr: sentinel, @@ -232,24 +241,25 @@ func TestAdoptMovedTrack_ToleratesQueryErrors(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { s := testScanner(t) - if s.adoptMovedTrack(context.Background(), tc.q, newPath, 1000, 2000, "rec-mbid") { + if s.adoptMovedTrack(context.Background(), tc.q, newPath, audioHash(1), "rec-mbid") { t.Error("reported adoption despite a query error") } }) } } -// A failed MBID lookup must not stop the fingerprint from being tried. -func TestAdoptMovedTrack_MbidErrorStillTriesFingerprint(t *testing.T) { +// A failed MBID lookup must not stop the audio hash from being tried. +func TestAdoptMovedTrack_MbidErrorStillTriesAudioHash(t *testing.T) { s := testScanner(t) q := &fakeAdopter{ - mbidErr: errors.New("db hiccup"), - byFingerprint: []dbq.FindMissingTrackByFingerprintRow{fpRow(9, oldPath)}, - adoptRows: 1, + mbidErr: errors.New("db hiccup"), + hash: audioHash(9), + byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(9, oldPath)}, + adoptRows: 1, } - if !s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "rec-mbid") { - t.Fatal("expected the fingerprint to be tried after an MBID lookup error") + if !s.adoptMovedTrack(context.Background(), q, newPath, audioHash(9), "rec-mbid") { + t.Fatal("expected the audio hash to be tried after an MBID lookup error") } if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(9) { t.Errorf("adopted = %+v, want row 9", q.adopted) @@ -280,9 +290,9 @@ func TestRowConverters(t *testing.T) { if len(got) != 2 || got[0].id != testUUID(1) || got[1].filePath != "/b" { t.Errorf("rowsFromMbid = %+v", got) } - got = rowsFromFingerprint([]dbq.FindMissingTrackByFingerprintRow{fpRow(3, "/c")}) + got = rowsFromAudioHash([]dbq.FindMissingTrackByAudioHashRow{hashRow(3, "/c")}) if len(got) != 1 || got[0].id != testUUID(3) || got[0].filePath != "/c" { - t.Errorf("rowsFromFingerprint = %+v", got) + t.Errorf("rowsFromAudioHash = %+v", got) } } diff --git a/internal/library/scanner.go b/internal/library/scanner.go index 28770e89..f2c00c32 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -79,11 +79,14 @@ type Scanner struct { // integration test can substitute a deterministic one: CI has no real audio // to fingerprint, and what the test pins is WHEN the scan fingerprints, not // what the tools print. Call it through fingerprintFile. - fingerprint func(ctx context.Context, path string) fingerprintResult + fingerprint func(ctx context.Context, path string, opts fingerprintOptions) fingerprintResult + // settings is the operator's fingerprinting policy (#3913), shared with the + // workers and the admin API. Nil fingerprints with the defaults. + settings *FingerprintSettingsService } -func New(pool *pgxpool.Pool, logger *slog.Logger, paths []string) *Scanner { - return &Scanner{pool: pool, logger: logger, paths: paths, fingerprint: computeFingerprint} +func New(pool *pgxpool.Pool, logger *slog.Logger, paths []string, settings *FingerprintSettingsService) *Scanner { + return &Scanner{pool: pool, logger: logger, paths: paths, fingerprint: computeFingerprint, settings: settings} } // Scan walks every configured root and upserts any audio file whose mtime is @@ -313,10 +316,18 @@ func (s *Scanner) scanFile( // // Computed before move adoption so adoption can match on the audio hash // (#3914); stored after the upsert, once the row id is known. + // + // With fingerprinting switched off (#3913) the scan decodes nothing, but it + // still takes the stream hash: a demux rather than a decode, and the only + // thing that recognises a moved file with no MBID. var fp fingerprintResult + fpCfg := s.settings.Get() fingerprinted := !unchanged if fingerprinted { - fp = s.fingerprintFile(ctx, path) + fp = s.fingerprintFile(ctx, path, fingerprintOptions{ + lengthSec: fpCfg.ChromaprintLengthSec, + chromaprint: fpCfg.Enabled, + }) } // A path we've never seen might not be a new track — it might be one that @@ -325,11 +336,11 @@ func (s *Scanner) scanFile( // file_path and updates THAT row: same track id, likes and play history // intact. Without this, renumbering an album forks every track on it. // - // Runs here rather than earlier because the fingerprint needs the probed - // duration, and only for genuinely unknown paths — a known path is already - // the row we're going to update. + // Runs after fingerprinting because, for a file with no MBID, adoption matches + // on its audio hash (#3914), and only for genuinely unknown paths — a known + // path is already the row we're going to update. if !knownTrack { - if s.adoptMovedTrack(ctx, q, path, info.Size(), durationMs, recordingMBID) { + if s.adoptMovedTrack(ctx, q, path, fp.streamSHA256, recordingMBID) { // Count it as an update: the row existed, and reporting it as Added // would overstate library growth on every reorganisation. knownTrack = true @@ -384,7 +395,13 @@ func (s *Scanner) scanFile( s.logger.Warn("library scan: LogChange track upsert failed", "track_id", track.ID, "err", err) } if fingerprinted { - storeFingerprint(ctx, q, s.logger, track.ID, path, fp) + if fpCfg.Enabled { + storeFingerprint(ctx, q, s.logger, track.ID, path, fp, fpCfg.ChromaprintLengthSec) + } else if err := q.DeleteTrackFingerprint(ctx, track.ID); err != nil { + // Off, nothing is stored — but a row describing the previous bytes + // must not outlive them, or the sweep would compare audio that is gone. + s.logger.Warn("fingerprint: clearing stale fingerprint failed", "path", path, "err", err) + } } if knownTrack { diff --git a/internal/library/scanner_test.go b/internal/library/scanner_test.go index 34f9637e..b17741a9 100644 --- a/internal/library/scanner_test.go +++ b/internal/library/scanner_test.go @@ -81,7 +81,7 @@ func TestScanner_Integration(t *testing.T) { "TIT2": "Solo", "TPE1": "The Artist Y", "TALB": "Y Album", "TRCK": "1", }) - scanner := New(pool, logger, []string{root}) + scanner := New(pool, logger, []string{root}, nil) stats, err := scanner.Scan(ctx, nil) if err != nil { t.Fatalf("first scan: %v", err) @@ -211,9 +211,9 @@ func writeTestMP3(t *testing.T, path string, frames map[string]string) { // playlist memberships travel with it — rather than forking into a marked ghost // plus a fresh zero-history row. // -// Uses the MBID path. The synthetic MP3s here carry no real audio, so ffprobe -// yields duration 0 and the size+duration fingerprint is deliberately unusable — -// which is why the recording MBID is the signal under test. +// Uses the MBID path. The synthetic MP3s here carry no real audio, so their audio +// stream hash cannot be relied on — which is why the recording MBID is the signal +// under test. The audio-hash path is covered by moved_test.go. // // Eight tracks with one rename keeps the marked fraction at 12.5%, under // missingMarkMaxFraction. That is load-bearing: if the rename exceeded the cap, @@ -259,7 +259,7 @@ func TestScanner_AdoptsMovedFile_Integration(t *testing.T) { }) } - scanner := New(pool, logger, []string{root}) + scanner := New(pool, logger, []string{root}, nil) if _, err := scanner.Scan(ctx, nil); err != nil { t.Fatalf("first scan: %v", err) } diff --git a/internal/server/server.go b/internal/server/server.go index 37b13da2..94c59ddd 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -100,6 +100,16 @@ type Server struct { // and serves the admin tuning endpoints from it. Router() constructs // a fallback when nil (tests). RecSettings *recsettings.Service + // FingerprintSettings is the DB-backed fingerprinting policy (M400 #3913). + // Constructed in cmd/minstrel/main.go and shared with the scanner and the + // fingerprint workers, so a save from the admin card reaches them without a + // restart. Router() constructs a fallback when nil (tests). + FingerprintSettings *library.FingerprintSettingsService + // ReacqSettings is the DB-backed missing-file re-acquisition policy + // (milestone #290) — the same instance the sweeper in cmd/minstrel/main.go + // reads, so a save from the admin card reaches it without a restart + // (#3936). Router() constructs a fallback when nil (tests). + ReacqSettings *reacquisition.SettingsService // StreamSecret is the HMAC key used by /api/cast/stream-token to // mint signed UPnP / Sonos stream URLs and by /api/tracks/{id}/stream // to verify them. Sourced from config.Config.StreamSecret. Tests that @@ -152,13 +162,18 @@ func (s *Server) Router() http.Handler { return lidarr.NewClient(cfg.BaseURL, cfg.APIKey) } lidarrReqs := lidarrrequests.NewService(s.Pool, lidarrCfg, lidarrClientFn, nil) - // Always usable even when the load fails — it falls back to the - // shipped defaults rather than leaving the admin card unable to - // render (same posture as netsettings above). - reacqSettings, raErr := reacquisition.NewSettingsService( - context.Background(), s.Pool, s.Logger) - if raErr != nil { - s.Logger.Warn("reacquisition settings unavailable; serving defaults", "err", raErr) + reacqSettings := s.ReacqSettings + if reacqSettings == nil { + // Test contexts construct Server without main.go's boot wiring. + // Always usable even when the load fails — it falls back to the + // shipped defaults rather than leaving the admin card unable to + // render (same posture as netsettings above). + var raErr error + reacqSettings, raErr = reacquisition.NewSettingsService( + context.Background(), s.Pool, s.Logger) + if raErr != nil { + s.Logger.Warn("reacquisition settings unavailable; serving defaults", "err", raErr) + } } lidarrQuar := lidarrquarantine.NewService(s.Pool, lidarrCfg, lidarrClientFn, s.DataDir) tracksSvc := tracks.NewService(s.Pool, s.Logger, lidarrUnmonitorAdapter{fn: lidarrClientFn}, s.DataDir) @@ -186,7 +201,17 @@ func (s *Server) Router() http.Handler { s.Logger.Error("server: recsettings boot failed", "err", err) } } - api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, recSettings, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.TagSettings, s.LibraryScanner, s.ScanCfg, s.DataDir, smtpSender, bus, s.PlaylistScheduler, s.StreamSecret, netSettings, reacqSettings) + fpSettings := s.FingerprintSettings + if fpSettings == nil { + // Test contexts construct Server without main.go's wiring. Always + // usable: a failed load serves the defaults. + var err error + fpSettings, err = library.NewFingerprintSettingsService(context.Background(), s.Pool) + if err != nil { + s.Logger.Warn("fingerprint settings unavailable; serving defaults", "err", err) + } + } + api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, recSettings, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.TagSettings, s.LibraryScanner, s.ScanCfg, s.DataDir, smtpSender, bus, s.PlaylistScheduler, s.StreamSecret, netSettings, reacqSettings, fpSettings) // /api/admin/scan is the only admin route owned by the server package // (it needs the Scanner). Register it as a single inline-middleware // route — using r.Route("/api/admin", ...) here would create a second diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 22e352a3..29c0eaf1 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -1,6 +1,7 @@ package server import ( + "bytes" "context" "encoding/json" "io" @@ -20,6 +21,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/library" + "git.fabledsword.com/bvandeusen/minstrel/internal/reacquisition" "git.fabledsword.com/bvandeusen/minstrel/internal/subsonic" ) @@ -242,6 +244,110 @@ func TestRouter_AdminSubtreeNotShadowed(t *testing.T) { } } +// TestRouter_ReacquisitionSettingsSavedThroughTheAPIReachTheSweeper is a +// regression test for #3936. Router() used to construct a +// reacquisition.SettingsService of its own, so a save from the admin card +// refreshed THAT instance's cache while the sweeper in cmd/minstrel/main.go +// kept serving what it had loaded at boot. The card showed the new policy, the +// feature kept running the old one, and only a restart reconciled them — the +// exact thing rule 25 says a setting must not need. +// +// The assertion is made against the instance main.go hands the sweeper: save +// through the router, then read that instance. A second service leaves it stale. +func TestRouter_ReacquisitionSettingsSavedThroughTheAPIReachTheSweeper(t *testing.T) { + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + if err := db.Migrate(dsn, logger); err != nil { + t.Fatalf("migrate: %v", err) + } + pool, err := pgxpool.New(context.Background(), dsn) + if err != nil { + t.Fatalf("pool: %v", err) + } + t.Cleanup(pool.Close) + + ctx := context.Background() + q := dbq.New(pool) + _, _ = pool.Exec(ctx, "DELETE FROM sessions WHERE user_agent = 'reacq-settings-test'") + _, _ = pool.Exec(ctx, "DELETE FROM users WHERE username = 'test-reacq-settings-admin'") + user, err := q.CreateUser(ctx, dbq.CreateUserParams{ + Username: "test-reacq-settings-admin", + PasswordHash: "x", + ApiToken: "test-reacq-settings-token", + IsAdmin: true, + }) + if err != nil { + t.Fatalf("CreateUser: %v", err) + } + t.Cleanup(func() { _, _ = pool.Exec(ctx, "DELETE FROM users WHERE id = $1", user.ID) }) + token := "reacq-settings-test-" + time.Now().Format("20060102150405.000000") + tokenHash := auth.HashSessionToken(token) + if _, err := pool.Exec(ctx, + "INSERT INTO sessions (user_id, token_hash, user_agent) VALUES ($1, $2, 'reacq-settings-test')", + user.ID, tokenHash[:], + ); err != nil { + t.Fatalf("insert session: %v", err) + } + + // The sweeper's service. Nothing else in the process may write to the + // settings for the assertion below to mean what it says. + sweeperSettings, err := reacquisition.NewSettingsService(ctx, pool, logger) + if err != nil { + t.Fatalf("reacquisition settings: %v", err) + } + before := sweeperSettings.Get() + t.Cleanup(func() { + if _, err := sweeperSettings.Set(context.Background(), before); err != nil { + t.Errorf("restore reacquisition settings: %v", err) + } + }) + wantGrace := before.GraceHours + 1 + if wantGrace > 720 { + wantGrace = before.GraceHours - 1 + } + + s := New(logger, pool, stubScanner{}, subsonic.Config{}, config.EventsConfig{}, + config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{}) + s.ReacqSettings = sweeperSettings + ts := httptest.NewServer(s.Router()) + defer ts.Close() + + body, err := json.Marshal(map[string]any{ + "enabled": before.Enabled, + "grace_hours": wantGrace, + "backoff_base_hours": before.BackoffBaseHours, + "backoff_max_hours": before.BackoffMaxHours, + "max_attempts": before.MaxAttempts, + "max_per_pass": before.MaxPerPass, + "auto_approve": before.AutoApprove, + }) + if err != nil { + t.Fatalf("marshal: %v", err) + } + req, err := http.NewRequest(http.MethodPut, ts.URL+"/api/admin/library/reacquisition", bytes.NewReader(body)) + if err != nil { + t.Fatalf("build request: %v", err) + } + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Content-Type", "application/json") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("PUT reacquisition settings: %v", err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + t.Fatalf("PUT reacquisition settings: status = %d, want 200", resp.StatusCode) + } + + if got := sweeperSettings.Get().GraceHours; got != wantGrace { + t.Fatalf("the sweeper's settings hold grace_hours = %d after the save, want %d — "+ + "the API wrote through a different service instance", got, wantGrace) + } +} + // stubScanner is a no-op ScanTrigger used only to make Server.Router() // register /api/admin/scan. Its Scan method must never be called by the // route-presence assertions in this file. diff --git a/internal/tracks/merge_test.go b/internal/tracks/merge_test.go new file mode 100644 index 00000000..c83d5ba7 --- /dev/null +++ b/internal/tracks/merge_test.go @@ -0,0 +1,39 @@ +package tracks + +import ( + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/library" +) + +func mbids(track, album string) library.MergedCopy { + c := library.MergedCopy{} + if track != "" { + c.TrackMbid = &track + } + if album != "" { + c.AlbumMbid = &album + } + return c +} + +// Unmonitoring a second file of the kept copy's own album track would stop +// Lidarr managing the kept file too; that case must be recognised. +func TestSameLidarrTrack(t *testing.T) { + for _, tc := range []struct { + name string + kept library.MergedCopy + removed library.MergedCopy + wantSame bool + }{ + {"same recording on the same album", mbids("rec", "alb"), mbids("rec", "alb"), true}, + {"same recording on a compilation", mbids("rec", "alb"), mbids("rec", "comp"), false}, + {"different recordings on one album", mbids("rec", "alb"), mbids("rec-2", "alb"), false}, + {"kept copy has no mbids", mbids("", ""), mbids("rec", "alb"), false}, + {"removed copy has no album mbid", mbids("rec", "alb"), mbids("rec", ""), false}, + } { + if got := sameLidarrTrack(tc.kept, tc.removed); got != tc.wantSame { + t.Errorf("%s: sameLidarrTrack = %v, want %v", tc.name, got, tc.wantSame) + } + } +} diff --git a/internal/tracks/service.go b/internal/tracks/service.go index 7abb98ec..1d576bd1 100644 --- a/internal/tracks/service.go +++ b/internal/tracks/service.go @@ -28,8 +28,10 @@ import ( "github.com/jackc/pgx/v5/pgxpool" "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/audit" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/library" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) // ErrNotFound is returned when the track id doesn't resolve. Aliased @@ -147,3 +149,70 @@ func (s *Service) RemoveTrack( return deleted.AlbumID, deleted.ArtistID, lidarrUnmonitorFailed, nil } + +// MergeDuplicates merges a duplicate group into the copy to keep +// (library.MergeDuplicateGroup), then — when asked — tells Lidarr to stop +// monitoring the removed copies so it does not download them again, and records +// the merge in the audit log. +// +// lidarrUnmonitorFailed reports that unmonitoring was asked for and at least one +// removed copy could not be unmonitored. Like RemoveTrack, that never fails the +// merge: the files and rows are already gone. +func (s *Service) MergeDuplicates( + ctx context.Context, groupID, survivorID, actorID pgtype.UUID, unmonitor bool, +) (res library.MergeResult, lidarrUnmonitorFailed bool, err error) { + res, err = library.MergeDuplicateGroup(ctx, s.pool, s.logger, s.dataDir, groupID, survivorID) + if err != nil { + return res, false, err + } + + if unmonitor && s.lidarr != nil { + for _, removed := range res.Removed { + if sameLidarrTrack(res.Survivor, removed) { + continue + } + if !hasLidarrIdentity(removed) { + s.logger.Warn("duplicate merge: lidarr unmonitor skipped — removed copy has no mbids", + "track_id", syncpkg.FormatUUID(removed.TrackID)) + lidarrUnmonitorFailed = true + continue + } + if uerr := s.lidarr.UnmonitorTrack(ctx, *removed.TrackMbid, *removed.AlbumMbid); uerr != nil { + s.logger.Warn("duplicate merge: lidarr unmonitor failed", + "track_id", syncpkg.FormatUUID(removed.TrackID), "err", uerr) + lidarrUnmonitorFailed = true + } + } + } + + removed := make([]map[string]string, 0, len(res.Removed)) + for _, c := range res.Removed { + removed = append(removed, map[string]string{"track_id": syncpkg.FormatUUID(c.TrackID), "file_path": c.FilePath}) + } + audit.WriteOrLog(ctx, s.pool, s.logger, actorID, pgtype.UUID{}, audit.ActionDuplicateMerge, map[string]any{ + "group_id": syncpkg.FormatUUID(groupID), + "tier": res.Tier, + "survivor_track_id": syncpkg.FormatUUID(res.Survivor.TrackID), + "survivor_path": res.Survivor.FilePath, + "removed": removed, + "moved": map[string]any{ + "play_events": res.PlayEvents, "skip_events": res.SkipEvents, + "likes": res.Likes, "playlist_entries": res.PlaylistEntries, + }, + }) + return res, lidarrUnmonitorFailed, nil +} + +// sameLidarrTrack reports whether two copies are the same Lidarr track: one +// recording on one album. Lidarr monitors per album track, so when the removed +// copy is a second file of the kept copy's own album track — the #3885 case — +// unmonitoring it would also stop Lidarr managing the file the operator chose to +// keep. Those are skipped, and are not a failure. +func sameLidarrTrack(a, b library.MergedCopy) bool { + return hasLidarrIdentity(a) && hasLidarrIdentity(b) && + *a.TrackMbid == *b.TrackMbid && *a.AlbumMbid == *b.AlbumMbid +} + +func hasLidarrIdentity(c library.MergedCopy) bool { + return c.TrackMbid != nil && *c.TrackMbid != "" && c.AlbumMbid != nil && *c.AlbumMbid != "" +} diff --git a/web/src/lib/api/admin.duplicates.test.ts b/web/src/lib/api/admin.duplicates.test.ts new file mode 100644 index 00000000..5be0cac9 --- /dev/null +++ b/web/src/lib/api/admin.duplicates.test.ts @@ -0,0 +1,39 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { dismissDuplicateGroup, listDuplicates, mergeDuplicateGroup, runDuplicateSweep } from './admin'; + +vi.mock('./client', () => ({ + api: { get: vi.fn(), post: vi.fn() } +})); + +import { api } from './client'; + +describe('admin duplicates API', () => { + beforeEach(() => vi.clearAllMocks()); + + it('listDuplicates GETs the paged report', async () => { + (api.get as unknown as ReturnType).mockResolvedValueOnce({ groups: [] }); + await listDuplicates(25, 25); + expect(api.get).toHaveBeenCalledWith('/api/admin/library/duplicates?limit=25&offset=25'); + }); + + it('runDuplicateSweep POSTs the trigger', async () => { + (api.post as unknown as ReturnType).mockResolvedValueOnce({ started: true }); + await runDuplicateSweep(); + expect(api.post).toHaveBeenCalledWith('/api/admin/library/duplicates/sweep', {}); + }); + + it('dismissDuplicateGroup POSTs to the group', async () => { + (api.post as unknown as ReturnType).mockResolvedValueOnce(undefined); + await dismissDuplicateGroup('g/1'); + expect(api.post).toHaveBeenCalledWith('/api/admin/library/duplicates/g%2F1/dismiss', {}); + }); + + it('mergeDuplicateGroup POSTs the chosen survivor', async () => { + (api.post as unknown as ReturnType).mockResolvedValueOnce({ removed_paths: [] }); + await mergeDuplicateGroup('g-1', { survivor_track_id: 't-1', unmonitor: true }); + expect(api.post).toHaveBeenCalledWith('/api/admin/library/duplicates/g-1/merge', { + survivor_track_id: 't-1', + unmonitor: true + }); + }); +}); diff --git a/web/src/lib/api/admin.fingerprints.test.ts b/web/src/lib/api/admin.fingerprints.test.ts index 9442fe45..1ae3669c 100644 --- a/web/src/lib/api/admin.fingerprints.test.ts +++ b/web/src/lib/api/admin.fingerprints.test.ts @@ -1,8 +1,14 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { getFingerprintCoverage, type FingerprintCoverage } from './admin'; +import { + getFingerprintCoverage, + getFingerprintSettings, + updateFingerprintSettings, + type FingerprintCoverage, + type FingerprintSettings +} from './admin'; vi.mock('./client', () => ({ - api: { get: vi.fn(), post: vi.fn() } + api: { get: vi.fn(), post: vi.fn(), put: vi.fn() } })); import { api } from './client'; @@ -15,7 +21,8 @@ describe('admin fingerprint coverage API', () => { total: 18026, fingerprinted: 9400, rejected: 12, - pending: 8614 + pending: 8614, + enabled: true }; (api.get as unknown as ReturnType).mockResolvedValueOnce(sample); const got = await getFingerprintCoverage(); @@ -24,9 +31,31 @@ describe('admin fingerprint coverage API', () => { }); it('buckets sum to the total', async () => { - const sample: FingerprintCoverage = { total: 10, fingerprinted: 6, rejected: 1, pending: 3 }; + const sample: FingerprintCoverage = { + total: 10, + fingerprinted: 6, + rejected: 1, + pending: 3, + enabled: true + }; (api.get as unknown as ReturnType).mockResolvedValueOnce(sample); const got = await getFingerprintCoverage(); expect(got.fingerprinted + got.rejected + got.pending).toBe(got.total); }); + + it('reads and saves the fingerprinting settings at one path', async () => { + const settings: FingerprintSettings = { + enabled: true, + chromaprint_length_sec: 120, + acoustic_max_bit_error_rate: 0.15, + backfill_concurrency: 2, + sweep_interval_hours: 1 + }; + (api.get as unknown as ReturnType).mockResolvedValueOnce(settings); + (api.put as unknown as ReturnType).mockResolvedValueOnce(settings); + await getFingerprintSettings(); + await updateFingerprintSettings(settings); + expect(api.get).toHaveBeenCalledWith('/api/admin/library/fingerprint-settings'); + expect(api.put).toHaveBeenCalledWith('/api/admin/library/fingerprint-settings', settings); + }); }); diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index f4e209e0..8f97e103 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -4,6 +4,8 @@ import { qk } from './queries'; import type { ActionResult, AdminMissingResponse, + AdminDuplicatesResponse, + MergeDuplicateResult, AdminPlaybackError, AdminQuarantineRow, LidarrConfig, @@ -320,6 +322,9 @@ export type FingerprintCoverage = { fingerprinted: number; rejected: number; pending: number; + // False when the operator has switched fingerprinting off (#3913): pending + // then never shrinks, and nothing should read as progress. + enabled: boolean; }; export async function getFingerprintCoverage(): Promise { @@ -337,6 +342,28 @@ export function createFingerprintCoverageQuery() { }); } +// Fingerprinting settings (#3913) ------------------------------------------ + +export type FingerprintSettings = { + enabled: boolean; + chromaprint_length_sec: number; + // The share of fingerprint bits two copies may disagree on and still be + // proposed as one recording. The card shows it as a match percentage. + acoustic_max_bit_error_rate: number; + backfill_concurrency: number; + sweep_interval_hours: number; +}; + +export async function getFingerprintSettings(): Promise { + return api.get('/api/admin/library/fingerprint-settings'); +} + +export async function updateFingerprintSettings( + s: FingerprintSettings +): Promise { + return api.put('/api/admin/library/fingerprint-settings', s); +} + // Cover-art providers ------------------------------------------------------ export type CoverProviderCapability = 'album_cover' | 'artist_thumb' | 'artist_fanart'; @@ -695,6 +722,50 @@ export async function updateNetworkSettings(hops: number): Promise { + return api.get( + `/api/admin/library/duplicates?limit=${limit}&offset=${offset}` + ); +} + +// Takes a plain offset, like createMissingFilesQuery, so a $derived caller +// re-creates the query on paging. Polls while a sweep might be running: the +// page is where the operator waits for one to finish. +export function createDuplicatesQuery(offset: number = 0, limit: number = 25) { + return createQuery({ + queryKey: qk.adminDuplicates(offset), + queryFn: () => listDuplicates(offset, limit), + staleTime: 30_000, + refetchInterval: 15_000 + }); +} + +export async function runDuplicateSweep(): Promise<{ started: boolean }> { + return api.post<{ started: boolean }>('/api/admin/library/duplicates/sweep', {}); +} + +// Merges a group into the copy chosen to keep, removing the other copies' files. +// survivor_track_id is the operator's choice; the server checks it belongs to +// the group. +export async function mergeDuplicateGroup( + id: string, + body: { survivor_track_id: string; unmonitor: boolean } +): Promise { + return api.post( + `/api/admin/library/duplicates/${encodeURIComponent(id)}/merge`, + body + ); +} + +export async function dismissDuplicateGroup(id: string): Promise { + await api.post(`/api/admin/library/duplicates/${encodeURIComponent(id)}/dismiss`, {}); +} + // Missing files (#2527) ----------------------------------------------------- export async function listMissingFiles( diff --git a/web/src/lib/api/errors.test.ts b/web/src/lib/api/errors.test.ts index d728c1fc..fc874041 100644 --- a/web/src/lib/api/errors.test.ts +++ b/web/src/lib/api/errors.test.ts @@ -75,6 +75,13 @@ describe('errMessage detail codes (#3918)', () => { ); }); + test('invalid_setting appends the field and range the server names', () => { + const msg = 'fingerprint setting out of range: chromaprint_length_sec must be 30-600'; + expect(errMessage({ code: 'invalid_setting', message: msg })).toBe( + `${ERROR_COPY.invalid_setting} ${msg}` + ); + }); + // Server messages are usually internal detail. Appending them for every code // would leak things like driver errors into toasts; this pins the scope. test('other codes never carry the server message', () => { diff --git a/web/src/lib/api/errors.ts b/web/src/lib/api/errors.ts index 1f09f55d..3de3b71b 100644 --- a/web/src/lib/api/errors.ts +++ b/web/src/lib/api/errors.ts @@ -15,7 +15,12 @@ export function errCode(err: unknown): string { * server messages are internal detail and must never reach a toast. Mirrored * in Android's ErrorCopy. */ -const DETAIL_CODES: ReadonlySet = new Set(['library_not_writable', 'file_delete_failed']); +const DETAIL_CODES: ReadonlySet = new Set([ + 'library_not_writable', + 'file_delete_failed', + // The server names the field and its range (#3913). + 'invalid_setting' +]); /** * Returns user-facing copy for an unknown error value. Looks up the diff --git a/web/src/lib/api/queries.ts b/web/src/lib/api/queries.ts index 7e62f0f4..105a308e 100644 --- a/web/src/lib/api/queries.ts +++ b/web/src/lib/api/queries.ts @@ -56,6 +56,8 @@ export const qk = { ['adminDiagnostics', f] as const, adminMissingFiles: (offset?: number) => ['adminMissingFiles', { offset: offset ?? 0 }] as const, + adminDuplicates: (offset?: number) => + ['adminDuplicates', { offset: offset ?? 0 }] as const, adminDiagnosticDevices: (userId?: string) => ['adminDiagnosticDevices', { userId: userId ?? 'all' }] as const, smtpConfig: () => ['smtpConfig'] as const, diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 51cab7fa..4648ee5d 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -418,3 +418,68 @@ export type AdminMissingResponse = { offset: number; groups: AdminMissingGroup[]; }; + +// Duplicates report (#3912) ------------------------------------------------- + +// One copy in a proposed duplicate group. like_count and play_count cover every +// user: they are what the operator weighs when choosing which copy to keep. +export type AdminDuplicateMember = { + track_id: string; + title: string; + artist_name: string; + album_id: string; + album_title: string; + file_path: string; + file_format: string; + file_size: number; + duration_sec: number; + added_at: string; + like_count: number; + play_count: number; +}; + +// exact: identical encoded audio. acoustic: the same recording, differently +// encoded; worst_bit_error_rate is the weakest link between any two members. +export type AdminDuplicateGroup = { + id: string; + tier: 'exact' | 'acoustic'; + worst_bit_error_rate: number | null; + detected_at: string; + survivor_track_id: string; + survivor_reason: string; + members: AdminDuplicateMember[]; +}; + +export type AdminDuplicateSweep = { + state: 'never' | 'running' | 'finished'; + started_at: string | null; + finished_at: string | null; + candidates: number | null; + groups_found: number | null; + oversize_clusters: number | null; + error_message: string | null; +}; + +// What a merge did (#3911). removed_paths are the files that were deleted from +// disk; lidarr_unmonitor_failed appears only when unmonitoring was asked for and +// failed. +export type MergeDuplicateResult = { + survivor_track_id: string; + removed_paths: string[]; + lidarr_unmonitor_failed?: boolean; +}; + +export type AdminDuplicatesResponse = { + sweep: AdminDuplicateSweep; + fingerprints: { + total: number; + fingerprinted: number; + rejected: number; + pending: number; + enabled: boolean; + }; + total: number; + limit: number; + offset: number; + groups: AdminDuplicateGroup[]; +}; diff --git a/web/src/lib/components/AdminTabs.svelte b/web/src/lib/components/AdminTabs.svelte index 1be389cf..9dff0ef1 100644 --- a/web/src/lib/components/AdminTabs.svelte +++ b/web/src/lib/components/AdminTabs.svelte @@ -9,6 +9,7 @@ { href: '/admin/requests', label: 'Requests' }, { href: '/admin/quarantine', label: 'Quarantine' }, { href: '/admin/missing-files', label: 'Missing files' }, + { href: '/admin/duplicates', label: 'Duplicates' }, { href: '/admin/playback-errors', label: 'Playback errors' }, { href: '/admin/diagnostics', label: 'Diagnostics' }, { href: '/admin/tuning', label: 'Tuning' }, diff --git a/web/src/lib/components/AdminTabs.test.ts b/web/src/lib/components/AdminTabs.test.ts index 85065bbc..c6e42c89 100644 --- a/web/src/lib/components/AdminTabs.test.ts +++ b/web/src/lib/components/AdminTabs.test.ts @@ -52,7 +52,7 @@ describe('AdminTabs', () => { ); }); - test('renders all nine tabs in order', () => { + test('renders all ten tabs in order', () => { state.pageUrl = new URL('http://localhost/admin'); render(AdminTabs); const links = screen.getAllByRole('link'); @@ -64,6 +64,9 @@ describe('AdminTabs', () => { // Missing files sits with Quarantine and Playback errors: the three // surfaces that show tracks needing an operator's attention. 'Missing files', + // Duplicates follows Missing files: both are library-health reports on + // what the library holds, rather than a queue of user reports. + 'Duplicates', 'Playback errors', 'Diagnostics', 'Tuning', diff --git a/web/src/lib/components/FingerprintSettingsCard.svelte b/web/src/lib/components/FingerprintSettingsCard.svelte new file mode 100644 index 00000000..85e766da --- /dev/null +++ b/web/src/lib/components/FingerprintSettingsCard.svelte @@ -0,0 +1,218 @@ + + +
+
+

Fingerprinting

+

+ How tracks are fingerprinted, and how alike two must sound to be proposed as the same + recording. Identical files are always found, whatever these say. +

+
+ + {#if loadError} +

+ Couldn't load fingerprinting settings. + +

+ {:else if form === null} +

Loading…

+ {:else} + + +
+ + + + + + + +
+ + {#if lengthChanged} +

+

+ {/if} + + {#if problems.length > 0} +
    + {#each problems as problem (problem)} +
  • {problem}
    • + {/each} +
+ {/if} + +
+ +
+ {/if} +
diff --git a/web/src/lib/components/FingerprintSettingsCard.test.ts b/web/src/lib/components/FingerprintSettingsCard.test.ts new file mode 100644 index 00000000..85ea770d --- /dev/null +++ b/web/src/lib/components/FingerprintSettingsCard.test.ts @@ -0,0 +1,141 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; +import type { FingerprintSettings } from '$lib/api/admin'; +import { ERROR_COPY } from '$lib/api/error-copy'; + +vi.mock('$lib/api/admin', () => ({ + getFingerprintSettings: vi.fn(), + updateFingerprintSettings: vi.fn() +})); + +vi.mock('$lib/stores/toast.svelte', () => ({ pushToast: vi.fn() })); + +import FingerprintSettingsCard from './FingerprintSettingsCard.svelte'; +import { getFingerprintSettings, updateFingerprintSettings } from '$lib/api/admin'; +import { pushToast } from '$lib/stores/toast.svelte'; + +const base: FingerprintSettings = { + enabled: true, + chromaprint_length_sec: 120, + acoustic_max_bit_error_rate: 0.15, + backfill_concurrency: 2, + sweep_interval_hours: 1 +}; + +afterEach(() => vi.clearAllMocks()); + +async function renderCard( + over: Partial = {}, + props: { libraryTotal?: number; onSaved?: () => void } = {} +) { + vi.mocked(getFingerprintSettings).mockResolvedValue({ ...base, ...over }); + const r = render(FingerprintSettingsCard, { props }); + await screen.findByRole('spinbutton', { name: /seconds of audio/i }); + return r; +} + +const saveButton = () => screen.getByRole('button', { name: /save/i }); + +describe('FingerprintSettingsCard', () => { + test('save is disabled until something changes', async () => { + await renderCard(); + expect(saveButton()).toHaveProperty('disabled', true); + await fireEvent.input(screen.getByRole('spinbutton', { name: /hours between sweeps/i }), { + target: { value: '6' } + }); + await waitFor(() => expect(saveButton()).toHaveProperty('disabled', false)); + }); + + // The matcher works in bit-error rates; the report shows match percentages. + // A card that showed 0.15 beside a report saying "96% match" would leave the + // operator converting in their head. + test('the threshold reads as a match percentage and saves as a bit-error rate', async () => { + vi.mocked(updateFingerprintSettings).mockResolvedValue({ + ...base, + acoustic_max_bit_error_rate: 0.1 + }); + await renderCard(); + const match = screen.getByRole('spinbutton', { name: /minimum match/i }) as HTMLInputElement; + expect(match.value).toBe('85'); + + await fireEvent.input(match, { target: { value: '90' } }); + await fireEvent.click(saveButton()); + await waitFor(() => + expect(updateFingerprintSettings).toHaveBeenCalledWith( + expect.objectContaining({ acoustic_max_bit_error_rate: 0.1 }) + ) + ); + }); + + // A new length re-fingerprints the whole library. Nothing else on the page + // would say so before the operator commits to it. + test('changing the length warns that every track is re-fingerprinted', async () => { + await renderCard({}, { libraryTotal: 18026 }); + expect(screen.queryByTestId('length-warning')).toBeNull(); + + await fireEvent.input(screen.getByRole('spinbutton', { name: /seconds of audio/i }), { + target: { value: '60' } + }); + const warning = await screen.findByTestId('length-warning'); + expect(warning.textContent).toMatch(/all\s+18,026\s+tracks/); + }); + + test('other changes carry no re-fingerprinting warning', async () => { + await renderCard({}, { libraryTotal: 18026 }); + await fireEvent.input(screen.getByRole('spinbutton', { name: /files fingerprinted at once/i }), { + target: { value: '4' } + }); + await waitFor(() => expect(saveButton()).toHaveProperty('disabled', false)); + expect(screen.queryByTestId('length-warning')).toBeNull(); + }); + + test('a value out of range blocks saving and says which', async () => { + await renderCard(); + await fireEvent.input(screen.getByRole('spinbutton', { name: /files fingerprinted at once/i }), { + target: { value: '12' } + }); + const problems = await screen.findByTestId('settings-problems'); + expect(problems.textContent).toMatch(/files fingerprinted at once must be from 1 to 8/i); + expect(saveButton()).toHaveProperty('disabled', true); + }); + + test('a rejected save surfaces the field the server names', async () => { + const message = 'fingerprint setting out of range: sweep_interval_hours must be 1-168'; + vi.mocked(updateFingerprintSettings).mockRejectedValue({ + code: 'invalid_setting', + message, + status: 400 + }); + await renderCard(); + await fireEvent.input(screen.getByRole('spinbutton', { name: /hours between sweeps/i }), { + target: { value: '6' } + }); + await fireEvent.click(saveButton()); + await waitFor(() => + expect(pushToast).toHaveBeenCalledWith(`${ERROR_COPY.invalid_setting} ${message}`, 'error') + ); + }); + + // Switching fingerprinting off changes what the page's counts mean. + test('a save tells the page, so its counts refresh', async () => { + const onSaved = vi.fn(); + vi.mocked(updateFingerprintSettings).mockResolvedValue({ ...base, enabled: false }); + await renderCard({}, { onSaved }); + + await fireEvent.click(screen.getByRole('checkbox', { name: /fingerprint tracks/i })); + await fireEvent.click(saveButton()); + await waitFor(() => expect(onSaved).toHaveBeenCalledTimes(1)); + expect(updateFingerprintSettings).toHaveBeenCalledWith( + expect.objectContaining({ enabled: false }) + ); + }); + + test('a failed load offers a retry rather than an empty card', async () => { + vi.mocked(getFingerprintSettings).mockRejectedValue(new Error('nope')); + render(FingerprintSettingsCard); + await waitFor(() => + expect(screen.getByText(/couldn't load fingerprinting settings/i)).toBeTruthy() + ); + expect(screen.getByRole('button', { name: /try again/i })).toBeTruthy(); + }); +}); diff --git a/web/src/lib/components/ReacquisitionSettingsCard.svelte b/web/src/lib/components/ReacquisitionSettingsCard.svelte index 9f37b1dc..2ae8c00a 100644 --- a/web/src/lib/components/ReacquisitionSettingsCard.svelte +++ b/web/src/lib/components/ReacquisitionSettingsCard.svelte @@ -6,6 +6,7 @@ updateReacquisitionSettings, type ReacquisitionSettings } from '$lib/api/admin'; + import { errMessage } from '$lib/api/errors'; import { pushToast } from '$lib/stores/toast.svelte'; // Policy for turning a missing file back into a Lidarr request @@ -60,8 +61,10 @@ } catch (e) { // The server validates the same ranges the database CHECKs enforce and // names the offending field, so surface its message rather than a - // generic failure. - pushToast(e instanceof Error ? e.message : "Couldn't save settings.", 'error'); + // generic failure. errMessage, not `e.message`: the API client throws a + // plain {code, message} object, never an Error, so an instanceof check + // here silently discarded every reason the server gave (#3937). + pushToast(errMessage(e), 'error'); } finally { saving = false; } diff --git a/web/src/lib/components/ReacquisitionSettingsCard.test.ts b/web/src/lib/components/ReacquisitionSettingsCard.test.ts index 7cfea1dd..2ede3ea9 100644 --- a/web/src/lib/components/ReacquisitionSettingsCard.test.ts +++ b/web/src/lib/components/ReacquisitionSettingsCard.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, test, vi } from 'vitest'; import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; import type { ReacquisitionSettings } from '$lib/api/admin'; +import { ERROR_COPY } from '$lib/api/error-copy'; vi.mock('$lib/api/admin', () => ({ getReacquisitionSettings: vi.fn(), @@ -80,10 +81,18 @@ describe('ReacquisitionSettingsCard', () => { // The server names the offending field ("grace_hours must be 1-720"); a // generic "couldn't save" would throw that away. + // + // Rejects with what api.put actually throws — a plain {code, message, status} + // object, not an Error. The old version of this test rejected with an Error, + // which no code path produces, and so passed while the card was discarding + // every server message it was handed (#3937). test('a rejected save surfaces the server message', async () => { - vi.mocked(updateReacquisitionSettings).mockRejectedValue( - new Error('grace_hours must be 1-720') - ); + const message = 'grace_hours must be 1-720'; + vi.mocked(updateReacquisitionSettings).mockRejectedValue({ + code: 'invalid_setting', + message, + status: 400 + }); await renderCard(); const grace = screen.getByRole('spinbutton', { name: /wait before the first attempt/i }); @@ -91,7 +100,7 @@ describe('ReacquisitionSettingsCard', () => { await fireEvent.click(await screen.findByRole('button', { name: /save/i })); await waitFor(() => - expect(pushToast).toHaveBeenCalledWith('grace_hours must be 1-720', 'error') + expect(pushToast).toHaveBeenCalledWith(`${ERROR_COPY.invalid_setting} ${message}`, 'error') ); }); diff --git a/web/src/lib/styles/error-copy.json b/web/src/lib/styles/error-copy.json index 80b94860..237d999a 100644 --- a/web/src/lib/styles/error-copy.json +++ b/web/src/lib/styles/error-copy.json @@ -42,6 +42,10 @@ "track_not_found": "That track no longer exists.", "library_not_writable": "The music library isn't writable by the server.", "file_delete_failed": "The file couldn't be deleted.", + "sweep_in_progress": "A duplicate sweep is already running.", + "duplicate_group_not_pending": "That group has already been resolved.", + "survivor_not_in_group": "That copy isn't part of this group any more.", + "invalid_setting": "That setting is out of range.", "album_not_found": "That album no longer exists.", "artist_not_found": "That artist no longer exists.", "playlist_not_found": "That playlist no longer exists.", diff --git a/web/src/routes/admin/+page.svelte b/web/src/routes/admin/+page.svelte index df5334ae..97feda2a 100644 --- a/web/src/routes/admin/+page.svelte +++ b/web/src/routes/admin/+page.svelte @@ -445,6 +445,11 @@ · {fingerprints.pending.toLocaleString()} pending {/if} + {#if fingerprints.enabled === false} + + · + fingerprinting is off + {/if} {#if fingerprints.rejected > 0} · + import { pageTitle } from '$lib/branding'; + import { Copy } from 'lucide-svelte'; + import { + createDuplicatesQuery, + runDuplicateSweep, + dismissDuplicateGroup, + mergeDuplicateGroup + } from '$lib/api/admin'; + import { errMessage } from '$lib/api/errors'; + import { pushToast } from '$lib/stores/toast.svelte'; + import { relativeTime } from '$lib/utils/relativeTime'; + import type { AdminDuplicateGroup, AdminDuplicateMember } from '$lib/api/types'; + import FingerprintSettingsCard from '$lib/components/FingerprintSettingsCard.svelte'; + + // Tracks the duplicate sweep believes hold one recording (#3912). Dismissing a + // group says "these are not duplicates", and the sweep will not propose that + // set again. Merging (#3911) keeps one copy, moves the others' likes, plays and + // playlist entries onto it, and deletes their files — so it asks twice. + + const PAGE_SIZE = 25; + + let offset = $state(0); + let sweeping = $state(false); + let dismissing = $state(null); + // Per group: which copy to keep (defaults to the proposed survivor), whether to + // unmonitor the removed copies in Lidarr, and the two-click confirm. + let keepChoice = $state>({}); + let unmonitorChoice = $state>({}); + let confirmingMerge = $state(null); + let merging = $state(null); + + const queryStore = $derived(createDuplicatesQuery(offset, PAGE_SIZE)); + const query = $derived($queryStore); + const data = $derived(query.data); + const groups = $derived((data?.groups ?? []) as AdminDuplicateGroup[]); + const total = $derived(data?.total ?? 0); + const hasMore = $derived(offset + groups.length < total); + const sweep = $derived(data?.sweep); + const prints = $derived(data?.fingerprints); + + async function onRunSweep() { + sweeping = true; + try { + await runDuplicateSweep(); + pushToast('Duplicate sweep started.'); + query.refetch(); + } catch (e: unknown) { + pushToast(errMessage(e), 'error'); + } finally { + sweeping = false; + } + } + + async function onDismiss(group: AdminDuplicateGroup) { + dismissing = group.id; + try { + await dismissDuplicateGroup(group.id); + pushToast('Marked as not duplicates.'); + query.refetch(); + } catch (e: unknown) { + pushToast(errMessage(e), 'error'); + } finally { + dismissing = null; + } + } + + function keeperOf(group: AdminDuplicateGroup): string { + return keepChoice[group.id] ?? group.survivor_track_id; + } + + function fileCountLabel(n: number): string { + return n === 1 ? '1 file' : `${n} files`; + } + + async function onMerge(group: AdminDuplicateGroup) { + // First click arms; the second, on the button that now names how many files + // go, does it. A merge deletes files, and nothing brings them back. + if (confirmingMerge !== group.id) { + confirmingMerge = group.id; + return; + } + confirmingMerge = null; + merging = group.id; + try { + const result = await mergeDuplicateGroup(group.id, { + survivor_track_id: keeperOf(group), + unmonitor: unmonitorChoice[group.id] ?? false + }); + pushToast(`Merged. Removed ${fileCountLabel(result.removed_paths.length)}.`); + if (result.lidarr_unmonitor_failed) { + pushToast("Merged, but Lidarr couldn't be told to stop monitoring the removed copies.", 'error'); + } + query.refetch(); + } catch (e: unknown) { + pushToast(errMessage(e), 'error'); + } finally { + merging = null; + } + } + + // "Identical audio" and "same recording" are different claims, and an + // operator deciding whether to merge needs to know which one they are + // looking at before anything else. + function tierLabel(g: AdminDuplicateGroup): string { + if (g.tier === 'exact') return 'Identical audio'; + const match = Math.round((1 - (g.worst_bit_error_rate ?? 0)) * 100); + return `Same recording · ${match}% match`; + } + + function sizeLabel(bytes: number): string { + return `${(bytes / 1_000_000).toFixed(1)} MB`; + } + + function durationLabel(sec: number): string { + const m = Math.floor(sec / 60); + const s = sec % 60; + return `${m}:${s.toString().padStart(2, '0')}`; + } + + function countLabel(n: number, one: string, many: string): string { + return n === 1 ? `1 ${one}` : `${n} ${many}`; + } + + // What the copy carries is the fact that decides which to keep. + function historyLabel(m: AdminDuplicateMember): string { + if (m.like_count === 0 && m.play_count === 0) return 'no likes or plays'; + return `${countLabel(m.like_count, 'like', 'likes')} · ${countLabel(m.play_count, 'play', 'plays')}`; + } + + +{pageTitle('Admin · Duplicates')} + +
+
+
+
+

Duplicates

+ {#if total > 0} + + {total} + + {/if} +
+ +
+

+ Tracks that hold the same recording more than once. Review each group; nothing is + merged or removed from here. +

+
+ + {#if sweep} +

+ {#if sweep.state === 'never'} + The duplicate sweep hasn't run yet. + {:else if sweep.state === 'running'} + Sweeping now — started {relativeTime(sweep.started_at ?? '')}. + {:else} + Last swept {relativeTime(sweep.finished_at ?? '')}, comparing + {(sweep.candidates ?? 0).toLocaleString()} tracks. + {#if sweep.error_message} + It stopped early: {sweep.error_message} + {/if} + {/if} + {#if prints && prints.pending > 0} + {#if prints.enabled === false} + Fingerprinting is off, so {prints.pending.toLocaleString()} tracks without a current + fingerprint aren't compared. + {:else} + {prints.pending.toLocaleString()} tracks are still waiting for a fingerprint and join + the comparison once they have one. + {/if} + {/if} +

+ {/if} + + {#if query.isPending} +

Loading duplicates…

+ {:else if query.isError} +

Couldn't load the duplicates report.

+ {:else if groups.length === 0} + +
+ + {#if prints && prints.total > 0 && prints.fingerprinted === 0} +

Nothing to compare yet.

+

+ {#if prints.enabled === false} + Fingerprinting is off, so no track has a fingerprint to compare. Turn it on in the + settings below. + {:else} + The library is still being fingerprinted — {prints.pending.toLocaleString()} tracks to go. + Duplicates appear here as the sweep finds them. + {/if} +

+ {:else if sweep?.state === 'never'} +

The sweep hasn't run yet.

+

+ It runs on its own whenever new fingerprints arrive, or now if you start it. +

+ {:else if sweep?.state === 'running'} +

Sweeping…

+

Anything it finds will appear here.

+ {:else} +

No duplicates found.

+

+ A group appears when two tracks hold identical audio, or the same recording in a + different encoding. Groups you dismiss don't come back. +

+ {/if} +
+ {:else} +
    + {#each groups as group (group.id)} +
  • +
    +
    +

    {tierLabel(group)}

    +

    Found {relativeTime(group.detected_at)}

    +
    +
    + + +
    +
    + + {#if confirmingMerge === group.id} + +
    +

    + The copy marked Keep stays. The other {fileCountLabel(group.members.length - 1)} will be + deleted from disk, and their likes, plays and playlist entries move to the copy kept. +

    + + +
    + {/if} + +
      + {#each group.members as m (m.track_id)} + {@const keep = m.track_id === group.survivor_track_id} +
    • + (keepChoice[group.id] = m.track_id)} + aria-label="Keep {m.title}, {m.file_format.toUpperCase()}, {m.file_path}" + /> +
      +
      + {m.title} + {#if keep} + + + Keep · {group.survivor_reason} + + {/if} +
      +
      + {m.artist_name} · {m.album_title} +
      +
      + {m.file_path} +
      +
      +
      +
      {m.file_format.toUpperCase()} · {sizeLabel(m.file_size)} · {durationLabel(m.duration_sec)}
      +
      {historyLabel(m)}
      +
      +
      • + {/each} +
    +
    • + {/each} +
+ + {#if hasMore || offset > 0} + + {/if} + {/if} + + + query.refetch()} /> +
diff --git a/web/src/routes/admin/duplicates/duplicates.test.ts b/web/src/routes/admin/duplicates/duplicates.test.ts new file mode 100644 index 00000000..c7c7fa60 --- /dev/null +++ b/web/src/routes/admin/duplicates/duplicates.test.ts @@ -0,0 +1,223 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { fireEvent, render, screen } from '@testing-library/svelte'; +import { mockQuery } from '../../../test-utils/query'; +import type { AdminDuplicatesResponse } from '$lib/api/types'; + +vi.mock('$lib/api/admin', () => ({ + createDuplicatesQuery: vi.fn(), + runDuplicateSweep: vi.fn().mockResolvedValue({ started: true }), + dismissDuplicateGroup: vi.fn().mockResolvedValue(undefined), + mergeDuplicateGroup: vi.fn().mockResolvedValue({ + survivor_track_id: 'www-01', + removed_paths: ['/music/Moe Shop/WWW (2020)/www-02.mp3'] + }), + // The page embeds FingerprintSettingsCard, which loads its own settings from + // this module; the card has its own suite. + getFingerprintSettings: vi.fn().mockResolvedValue({ + enabled: true, + chromaprint_length_sec: 120, + acoustic_max_bit_error_rate: 0.15, + backfill_concurrency: 2, + sweep_interval_hours: 1 + }), + updateFingerprintSettings: vi.fn() +})); + +import AdminDuplicatesPage from './+page.svelte'; +import { + createDuplicatesQuery, + dismissDuplicateGroup, + mergeDuplicateGroup, + runDuplicateSweep +} from '$lib/api/admin'; + +const HOUR = 3_600_000; +const ago = (ms: number) => new Date(Date.now() - ms).toISOString(); + +const finishedSweep = { + state: 'finished' as const, + started_at: ago(2 * HOUR), + finished_at: ago(HOUR), + candidates: 1200, + groups_found: 1, + oversize_clusters: 0, + error_message: null +}; +const allFingerprinted = { total: 1200, fingerprinted: 1200, rejected: 0, pending: 0, enabled: true }; + +function member(id: string, extra: Partial = {}) { + return { + track_id: id, + title: 'WWW', + artist_name: 'Moe Shop', + album_id: 'al-1', + album_title: 'WWW', + file_path: `/music/Moe Shop/WWW (2020)/${id}.mp3`, + file_format: 'mp3', + file_size: 6_900_000, + duration_sec: 215, + added_at: ago(500 * HOUR), + like_count: 0, + play_count: 0, + ...extra + }; +} + +function response(over: Partial = {}): AdminDuplicatesResponse { + return { + sweep: finishedSweep, + fingerprints: allFingerprinted, + total: 1, + limit: 25, + offset: 0, + groups: [ + { + id: 'g-1', + tier: 'exact', + worst_bit_error_rate: null, + detected_at: ago(HOUR), + survivor_track_id: 'www-01', + survivor_reason: 'in the library longest', + members: [member('www-01', { like_count: 2, play_count: 14 }), member('www-02')] + } + ], + ...over + }; +} + +function renderWith(data: AdminDuplicatesResponse | undefined) { + vi.mocked(createDuplicatesQuery).mockReturnValue( + mockQuery({ data }) as ReturnType + ); + return render(AdminDuplicatesPage); +} + +function text(el: HTMLElement): string { + return (el.textContent ?? '').replace(/\s+/g, ' ').trim(); +} + +afterEach(() => vi.clearAllMocks()); + +describe('admin duplicates', () => { + test('shows each group with its members, what they carry, and the copy to keep', () => { + renderWith(response()); + expect(screen.getAllByTestId('duplicate-group')).toHaveLength(1); + expect(screen.getAllByTestId('duplicate-member')).toHaveLength(2); + expect(text(screen.getByTestId('duplicate-tier'))).toBe('Identical audio'); + expect(text(screen.getByTestId('survivor-badge'))).toBe('Keep · in the library longest'); + const history = screen.getAllByTestId('member-history').map(text); + expect(history).toEqual(['2 likes · 14 plays', 'no likes or plays']); + }); + + // The badge must sit on the proposed survivor, not merely appear somewhere. + test('the keep badge is on the survivor row', () => { + renderWith(response()); + const rows = screen.getAllByTestId('duplicate-member'); + expect(rows[0].querySelector('[data-testid="survivor-badge"]')).not.toBeNull(); + expect(rows[1].querySelector('[data-testid="survivor-badge"]')).toBeNull(); + }); + + test('an acoustic group reads as a match percentage', () => { + const r = response(); + r.groups[0] = { ...r.groups[0], tier: 'acoustic', worst_bit_error_rate: 0.04 }; + renderWith(r); + expect(text(screen.getByTestId('duplicate-tier'))).toBe('Same recording · 96% match'); + }); + + // Three states all show zero groups. Each must say which it is. + test('empty while still fingerprinting says so', () => { + renderWith( + response({ + groups: [], + total: 0, + fingerprints: { total: 1200, fingerprinted: 0, rejected: 0, pending: 1200, enabled: true } + }) + ); + expect(text(screen.getByTestId('empty-state'))).toContain('still being fingerprinted'); + }); + + // Switched off, the backlog never shrinks. "Still being fingerprinted" would + // promise work nothing is doing (#3913). + test('with fingerprinting off the page says so instead of promising progress', () => { + renderWith( + response({ + groups: [], + total: 0, + fingerprints: { total: 1200, fingerprinted: 0, rejected: 0, pending: 1200, enabled: false } + }) + ); + const empty = text(screen.getByTestId('empty-state')); + expect(empty).toContain('Fingerprinting is off'); + expect(empty).not.toContain('still being fingerprinted'); + expect(text(screen.getByTestId('sweep-status'))).toContain('Fingerprinting is off'); + }); + + test('the fingerprinting settings are on this page', async () => { + renderWith(response()); + expect(await screen.findByRole('spinbutton', { name: /seconds of audio/i })).toBeTruthy(); + }); + + test('empty before any sweep says the sweep has not run', () => { + renderWith( + response({ + groups: [], + total: 0, + sweep: { ...finishedSweep, state: 'never', started_at: null, finished_at: null, candidates: null } + }) + ); + expect(text(screen.getByTestId('empty-state'))).toContain("hasn't run yet"); + }); + + test('empty after a sweep says no duplicates were found', () => { + renderWith(response({ groups: [], total: 0 })); + expect(text(screen.getByTestId('empty-state'))).toContain('No duplicates found'); + }); + + test('Not duplicates dismisses that group', async () => { + renderWith(response()); + await fireEvent.click(screen.getByRole('button', { name: 'Not duplicates' })); + expect(dismissDuplicateGroup).toHaveBeenCalledWith('g-1'); + }); + + test('Sweep now starts a sweep', async () => { + renderWith(response()); + await fireEvent.click(screen.getByRole('button', { name: 'Sweep now' })); + expect(runDuplicateSweep).toHaveBeenCalledTimes(1); + }); + + // A merge deletes files. The first click must only arm it. + test('Merge needs a second click, and keeps the proposed copy by default', async () => { + renderWith(response()); + await fireEvent.click(screen.getByRole('button', { name: 'Merge…' })); + expect(mergeDuplicateGroup).not.toHaveBeenCalled(); + expect(text(screen.getByTestId('merge-confirm'))).toContain('The other 1 file will be deleted from disk'); + + await fireEvent.click(screen.getByRole('button', { name: 'Remove 1 file and merge' })); + expect(mergeDuplicateGroup).toHaveBeenCalledWith('g-1', { + survivor_track_id: 'www-01', + unmonitor: false + }); + }); + + test('choosing another copy to keep sends that copy', async () => { + renderWith(response()); + const radios = screen.getAllByRole('radio'); + expect((radios[0] as HTMLInputElement).checked).toBe(true); + await fireEvent.click(radios[1]); + + await fireEvent.click(screen.getByRole('button', { name: 'Merge…' })); + await fireEvent.click(screen.getByRole('button', { name: 'Remove 1 file and merge' })); + expect(mergeDuplicateGroup).toHaveBeenCalledWith('g-1', { + survivor_track_id: 'www-02', + unmonitor: false + }); + }); + + test('Cancel disarms the merge', async () => { + renderWith(response()); + await fireEvent.click(screen.getByRole('button', { name: 'Merge…' })); + await fireEvent.click(screen.getByRole('button', { name: 'Cancel' })); + expect(screen.queryByTestId('merge-confirm')).toBeNull(); + expect(screen.getByRole('button', { name: 'Merge…' })).toBeTruthy(); + }); +});