feat(jobs): register the autotag apply, and ask before quitting
The apply was a bare goroutine whose progress lived in a component field discarded on navigation, with no cancel and no record of where it stopped if the app quit while it was rewriting tags — beside a registry that gives every other long-running operation exactly those things. `jobs.KindAutotagApply` now carries progress, a cancel wired to the apply's context, and a terminal state that tells cancelled from failed. `OnBeforeClose` returns false unconditionally today; it now asks while a file-writing job is in flight. Still not durable: quitting cancels cleanly but nothing records where it stopped for the next launch. That belongs with the deferred download/jobs work.
This commit is contained in:
+40
-1
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<void>;
|
||||
|
||||
@@ -35,6 +36,8 @@ export function SelectSearchCandidate(arg1:string,arg2:string,arg3:string):Promi
|
||||
|
||||
export function SetContext(arg1:context.Context):Promise<void>;
|
||||
|
||||
export function SetJobRegistry(arg1:jobs.Registry):Promise<void>;
|
||||
|
||||
export function Skip(arg1:string):Promise<void>;
|
||||
|
||||
export function SplitMixedFolder(arg1:string):Promise<Array<autotagservice.PendingItem>>;
|
||||
@@ -42,3 +45,5 @@ export function SplitMixedFolder(arg1:string):Promise<Array<autotagservice.Pendi
|
||||
export function StartAutotagQueue(arg1:number):Promise<void>;
|
||||
|
||||
export function StartBackgroundPrefetch():Promise<void>;
|
||||
|
||||
export function WritesInFlight():Promise<boolean>;
|
||||
|
||||
@@ -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']();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user