diff --git a/backend/app.go b/backend/app.go index f856725..861cd39 100644 --- a/backend/app.go +++ b/backend/app.go @@ -189,6 +189,7 @@ func NewYellowJacketApp( yjApp.explore, yjApp.tagWriter, ) + yjApp.autotag.SetJobRegistry(yjApp.jobs) // Create the download subsystem. Acquiring music is optional: a // failure here (unwritable data dir, say) must not stop the app @@ -494,8 +495,19 @@ func (yj *YellowJacketApp) OnStartup(ctx context.Context) { yj.player.SetMediaControls(yj.mediaControls) } -// OnBeforeClose captures window state while the window is still alive. +// OnBeforeClose captures window state while the window is still alive, +// and asks first when quitting would abandon a job that is writing to +// the user's files. +// +// Returning true keeps the window open. Quitting mid-apply cancels the +// service context and leaves a folder half-retagged with nothing +// recording where it stopped (errors.p4), which is the one case worth +// interrupting a quit for. func (yj *YellowJacketApp) OnBeforeClose(ctx context.Context) bool { + if yj.confirmQuitDuringWrites(ctx) { + return true + } + w, h := wailsruntime.WindowGetSize(ctx) // Guard against a bogus size clobbering a good saved one. During @@ -533,6 +545,33 @@ func (yj *YellowJacketApp) OnBeforeClose(ctx context.Context) bool { return false } +// confirmQuitDuringWrites returns true when the user chose to stay. +// A dialog that cannot be shown is not allowed to trap anyone in the +// app, so any error here quits. +func (yj *YellowJacketApp) confirmQuitDuringWrites(ctx context.Context) bool { + if yj.autotag == nil || !yj.autotag.WritesInFlight() { + return false + } + + answer, err := wailsruntime.MessageDialog(ctx, wailsruntime.MessageDialogOptions{ + Type: wailsruntime.QuestionDialog, + Title: "Tags are still being written", + Message: "YellowJacket is rewriting tags on your files. " + + "Quitting now leaves that folder holding a mix of old and " + + "new tags.\n\nQuit anyway?", + Buttons: []string{"Quit anyway", "Keep writing"}, + DefaultButton: "Keep writing", + CancelButton: "Keep writing", + }) + if err != nil { + yj.logger.Warn("could not ask about quitting mid-write", "err", err) + + return false + } + + return answer == "Keep writing" || answer == "No" +} + // OnShutdown saves player state and cleans up resources before the application exits. func (yj *YellowJacketApp) OnShutdown(_ context.Context) { if yj.player != nil { diff --git a/backend/autotagservice/apply_jobs.go b/backend/autotagservice/apply_jobs.go new file mode 100644 index 0000000..56c8105 --- /dev/null +++ b/backend/autotagservice/apply_jobs.go @@ -0,0 +1,84 @@ +package autotagservice + +import ( + "context" + "strings" + + "yellowjacket/backend/jobs" +) + +// applyJobPrefix namespaces autotag apply jobs in the shared registry. +const applyJobPrefix = "autotag:" + +// SetJobRegistry wires the background job registry so an apply reports +// progress and offers a cancel like every other long-running operation. +// +// Before this, apply was a bare goroutine whose progress lived in a +// component field that navigation discarded, with no cancel and no +// record of where it stopped (errors.C3). Everything routed through the +// registry gets progress, cancel and the global indicator for free; the +// three subsystems that lacked them were the three that were not +// registered. +func (s *Service) SetJobRegistry(reg *jobs.Registry) { + s.mu.Lock() + s.jobsReg = reg + s.mu.Unlock() +} + +// applyJobID is the registry ID for one folder's apply. +func applyJobID(groupKey string) string { + return applyJobPrefix + groupKey +} + +// WritesInFlight reports whether an apply is currently rewriting tags +// on disk. Quitting mid-apply leaves a folder half-retagged, so the app +// asks before closing (errors.p4). +func (s *Service) WritesInFlight() bool { + s.mu.Lock() + defer s.mu.Unlock() + + return len(s.runningApplies) > 0 +} + +// startApplyJob registers the job and returns the handle plus a context +// the user's Cancel button can stop. A nil registry (tests, and the +// window before wiring) degrades to the plain context. +func (s *Service) startApplyJob( + groupKey string, + total int, +) (*jobs.Handle, context.Context, context.CancelFunc) { + s.mu.Lock() + parent := s.ctx + reg := s.jobsReg + s.mu.Unlock() + + ctx, cancel := context.WithCancel(parent) + + if reg == nil { + return nil, ctx, cancel + } + + handle := reg.Start(jobs.Spec{ + ID: applyJobID(groupKey), + Kind: jobs.KindAutotagApply, + Title: "Writing tags", + Subtitle: folderLabel(groupKey), + Total: int64(total), + Caps: jobs.Caps{Cancellable: true}, + Controls: jobs.Controls{Cancel: cancel}, + }) + + return handle, ctx, cancel +} + +// folderLabel is the part of a group key worth showing: the folder, +// not the whole path, which is usually wider than the row. +func folderLabel(groupKey string) string { + trimmed := strings.TrimRight(groupKey, "/") + + if idx := strings.LastIndex(trimmed, "/"); idx >= 0 { + return trimmed[idx+1:] + } + + return trimmed +} diff --git a/backend/autotagservice/apply_jobs_test.go b/backend/autotagservice/apply_jobs_test.go new file mode 100644 index 0000000..59be6d3 --- /dev/null +++ b/backend/autotagservice/apply_jobs_test.go @@ -0,0 +1,143 @@ +package autotagservice + +import ( + "context" + "errors" + "log/slog" + "testing" + + "yellowjacket/backend/autotag" + "yellowjacket/backend/jobs" +) + +var errApplyFailed = errors.New("write failed") + +// newJobService builds the smallest Service that can register a job: +// no database, no scorer, no MB client. +func newJobService(t *testing.T) (*Service, *jobs.Registry) { + t.Helper() + + logger := slog.New(slog.DiscardHandler) + reg := jobs.NewRegistry(logger, nil) + svc := &Service{ + logger: logger, + ctx: context.Background(), + runningApplies: make(map[string]struct{}), + } + + svc.SetJobRegistry(reg) + + return svc, reg +} + +func TestApplyJob_RegistersACancellableJob(t *testing.T) { + svc, reg := newJobService(t) + + handle, ctx, cancel := svc.startApplyJob("/music/Artist/Album", 9) + defer cancel() + + if handle == nil { + t.Fatal("no job handle: an apply that is not registered has no cancel and no progress") + } + + snapshot := handle.Snapshot() + + if snapshot.Kind != jobs.KindAutotagApply { + t.Errorf("kind = %q, want %q", snapshot.Kind, jobs.KindAutotagApply) + } + + if snapshot.Total != 9 { + t.Errorf("total = %d, want 9", snapshot.Total) + } + + if !snapshot.Caps.Cancellable { + t.Error("apply job is not cancellable, which is the point of registering it") + } + + if snapshot.Subtitle != "Album" { + t.Errorf("subtitle = %q, want the folder name", snapshot.Subtitle) + } + + // The registry's Cancel control has to reach the context the apply + // is running under, or the button is decoration. + reg.Cancel(applyJobID("/music/Artist/Album")) + + <-ctx.Done() +} + +func TestApplyJob_FinishStateMatchesTheRun(t *testing.T) { + cancelled, cancelStop := context.WithCancel(context.Background()) + cancelStop() + + tests := []struct { + name string + ctx context.Context + result *autotag.ApplyResult + err error + want jobs.State + }{ + { + name: "every track written", + ctx: context.Background(), + result: &autotag.ApplyResult{Succeeded: 4}, + want: jobs.StateComplete, + }, + { + name: "some tracks failed", + ctx: context.Background(), + result: &autotag.ApplyResult{Succeeded: 3, Failed: 1}, + want: jobs.StateComplete, + }, + { + name: "the apply itself failed", + ctx: context.Background(), + err: errApplyFailed, + want: jobs.StateError, + }, + { + // Cancelled beats failed: Apply returns a context error on + // the way out, and reading that as a failure would make + // every cancel look like a bug. + name: "the user cancelled", + ctx: cancelled, + result: &autotag.ApplyResult{Succeeded: 1}, + err: context.Canceled, + want: jobs.StateCancelled, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, _ := newJobService(t) + handle, _, cancel := svc.startApplyJob("/music/"+tt.name, 4) + + defer cancel() + + svc.finishApplyJob(tt.ctx, handle, tt.result, tt.err) + + if got := handle.State(); got != tt.want { + t.Errorf("state = %q, want %q", got, tt.want) + } + }) + } +} + +func TestWritesInFlight_TracksTheApplySet(t *testing.T) { + svc, _ := newJobService(t) + + if svc.WritesInFlight() { + t.Fatal("nothing is running, so nothing should be reported in flight") + } + + svc.tryStartApply("/music/Album") + + if !svc.WritesInFlight() { + t.Error("an apply is running: quitting now would half-retag a folder") + } + + svc.endApply("/music/Album") + + if svc.WritesInFlight() { + t.Error("the apply finished and the app should stop asking about it") + } +} diff --git a/backend/autotagservice/service.go b/backend/autotagservice/service.go index 11d0313..13f7e6a 100644 --- a/backend/autotagservice/service.go +++ b/backend/autotagservice/service.go @@ -24,6 +24,7 @@ import ( "yellowjacket/backend/database/sql/sqlcgen" "yellowjacket/backend/events" "yellowjacket/backend/explore" + "yellowjacket/backend/jobs" "yellowjacket/backend/metadata" "yellowjacket/backend/tagwriter" ) @@ -84,6 +85,10 @@ type Service struct { logger *slog.Logger ctx context.Context + // Registry for the apply job, wired after construction like every + // other subsystem's. Guarded by mu. + jobsReg *jobs.Registry + // Queue cursor — the group_key of the last item returned. // GetNextPending uses it to advance. Reset by StartAutotagQueue. mu sync.Mutex @@ -1103,7 +1108,9 @@ func (s *Service) ApplyAsync(groupKey, releaseMBID string) error { "total": total, }) - go s.runApply(groupKey, plan, total) + handle, ctx, cancel := s.startApplyJob(groupKey, total) + + go s.runApply(ctx, cancel, handle, groupKey, plan, total) return nil } @@ -1147,10 +1154,26 @@ func (s *Service) prepareApplyPlan( // runApply executes the plan in the background and emits progress // + completion events. Always releases the in-flight slot when // it returns, even on panic. -func (s *Service) runApply(groupKey string, plan *autotag.ApplyPlan, total int) { +// +// The job handle is the same progress and cancel surface every other +// long-running operation has; the events stay because the autotag page +// drives its per-folder row from them. +func (s *Service) runApply( + ctx context.Context, + cancel context.CancelFunc, + handle *jobs.Handle, + groupKey string, + plan *autotag.ApplyPlan, + total int, +) { defer s.endApply(groupKey) + defer cancel() onProgress := func(current, total, succeeded, failed int) { + if handle != nil { + handle.SetProgress(int64(current), int64(total)) + } + s.emitEvent(events.AutotagApplyProgress, map[string]any{ "groupKey": groupKey, "current": current, @@ -1160,7 +1183,7 @@ func (s *Service) runApply(groupKey string, plan *autotag.ApplyPlan, total int) }) } - result, err := s.applier.Apply(s.ctx, plan, onProgress) + result, err := s.applier.Apply(ctx, plan, onProgress) finished := map[string]any{ "groupKey": groupKey, @@ -1178,9 +1201,40 @@ func (s *Service) runApply(groupKey string, plan *autotag.ApplyPlan, total int) finished["error"] = err.Error() } + s.finishApplyJob(ctx, handle, result, err) + s.emitEvent(events.AutotagApplyFinished, finished) } +// finishApplyJob closes the job out in the state the run ended in, so +// a cancelled apply reads as cancelled rather than as a failure and a +// partial write says how far it got. +func (s *Service) finishApplyJob( + ctx context.Context, + handle *jobs.Handle, + result *autotag.ApplyResult, + err error, +) { + if handle == nil { + return + } + + switch { + case ctx.Err() != nil: + handle.Cancelled() + case err != nil: + handle.Fail(err) + case result != nil && result.Failed > 0: + handle.Logf(jobs.LevelWarn, fmt.Sprintf( + "%d of %d tracks could not be written", + result.Failed, result.Succeeded+result.Failed, + )) + handle.Complete() + default: + handle.Complete() + } +} + // tryStartApply records that an Apply for the given group is // running. Returns false when a previous job for the same key // hasn't finished yet — caller should treat that as diff --git a/backend/jobs/jobs.go b/backend/jobs/jobs.go index 1f94956..0d80fad 100644 --- a/backend/jobs/jobs.go +++ b/backend/jobs/jobs.go @@ -22,9 +22,10 @@ type Kind string // Job kinds. const ( - KindLibraryScan Kind = "library-scan" - KindIndexBuild Kind = "index-build" - KindDownload Kind = "download" + KindLibraryScan Kind = "library-scan" + KindIndexBuild Kind = "index-build" + KindDownload Kind = "download" + KindAutotagApply Kind = "autotag-apply" ) // State is the lifecycle position of a job. diff --git a/frontend/wailsjs/go/autotagservice/Service.d.ts b/frontend/wailsjs/go/autotagservice/Service.d.ts index d5c375e..76fd614 100755 --- a/frontend/wailsjs/go/autotagservice/Service.d.ts +++ b/frontend/wailsjs/go/autotagservice/Service.d.ts @@ -2,6 +2,7 @@ // This file is automatically generated. DO NOT EDIT import {autotagservice} from '../models'; import {context} from '../models'; +import {jobs} from '../models'; export function AckLibraryWarning(arg1:number):Promise; @@ -35,6 +36,8 @@ export function SelectSearchCandidate(arg1:string,arg2:string,arg3:string):Promi export function SetContext(arg1:context.Context):Promise; +export function SetJobRegistry(arg1:jobs.Registry):Promise; + export function Skip(arg1:string):Promise; export function SplitMixedFolder(arg1:string):Promise>; @@ -42,3 +45,5 @@ export function SplitMixedFolder(arg1:string):Promise; export function StartBackgroundPrefetch():Promise; + +export function WritesInFlight():Promise; diff --git a/frontend/wailsjs/go/autotagservice/Service.js b/frontend/wailsjs/go/autotagservice/Service.js index 4515ed2..ff9fe74 100755 --- a/frontend/wailsjs/go/autotagservice/Service.js +++ b/frontend/wailsjs/go/autotagservice/Service.js @@ -66,6 +66,10 @@ export function SetContext(arg1) { return window['go']['autotagservice']['Service']['SetContext'](arg1); } +export function SetJobRegistry(arg1) { + return window['go']['autotagservice']['Service']['SetJobRegistry'](arg1); +} + export function Skip(arg1) { return window['go']['autotagservice']['Service']['Skip'](arg1); } @@ -81,3 +85,7 @@ export function StartAutotagQueue(arg1) { export function StartBackgroundPrefetch() { return window['go']['autotagservice']['Service']['StartBackgroundPrefetch'](); } + +export function WritesInFlight() { + return window['go']['autotagservice']['Service']['WritesInFlight'](); +}