Remove the organizer's Preview and apply section
The preview/apply flow on the Organize tab did not work as expected, so it is removed along with its backend: the stored per-pair preview, the /organize/preview, /organize/plan and /organize/apply endpoints and their UI and types. The File naming card stays, and renaming now happens only through "Organize automatically" during scans, which keeps the underlying planning/moving code (now internal), undo, crash recovery and dry run. The rename-folders warning is unchanged. Tests now drive the kept planning and moving code directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
dbcf562f7e
commit
96ea856dfd
8 files changed
+76
-462
No files matched your search
@@ -45,9 +45,6 @@ type Engine struct {
|
||||
tagTools tags.Tools
|
||||
tagCache tagCache
|
||||
|
||||
plansMu sync.Mutex
|
||||
plans map[int64]*OrganizePlan // latest organize preview per pair
|
||||
|
||||
jobsMu sync.Mutex
|
||||
jobs map[int64]*Job
|
||||
nextID int64
|
||||
|
||||
+22
-130
@@ -14,7 +14,6 @@ import (
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
"unicode"
|
||||
"unicode/utf8"
|
||||
|
||||
@@ -233,31 +232,26 @@ func (c *tagCache) get(ctx context.Context, t tags.Tools, abs string, in fsops.I
|
||||
|
||||
// ---- planning ----
|
||||
|
||||
// OrganizeItem is one file in an organize plan.
|
||||
type OrganizeItem struct {
|
||||
ID int `json:"id"`
|
||||
Side string `json:"side"`
|
||||
From string `json:"from"`
|
||||
To string `json:"to,omitempty"`
|
||||
Move bool `json:"move"`
|
||||
Reason string `json:"reason,omitempty"` // why it is skipped
|
||||
Sidecars []string `json:"sidecars,omitempty"` // e.g. lyrics files moving along
|
||||
// organizeItem is one file the organizer looked at.
|
||||
type organizeItem struct {
|
||||
Side string
|
||||
From string
|
||||
To string
|
||||
Move bool
|
||||
Reason string // why it is skipped
|
||||
Sidecars []string // e.g. lyrics files moving along
|
||||
|
||||
ino uint64
|
||||
size, mtime int64
|
||||
}
|
||||
|
||||
// OrganizePlan is the result of a preview.
|
||||
type OrganizePlan struct {
|
||||
PairID int64 `json:"pair_id"`
|
||||
Sides []string `json:"sides"`
|
||||
Template string `json:"template"`
|
||||
CreatedAt int64 `json:"created_at"`
|
||||
Scanned int `json:"scanned"`
|
||||
Conforming int `json:"conforming"`
|
||||
Moves int `json:"moves"`
|
||||
Skips map[string]int `json:"skips"`
|
||||
Items []OrganizeItem `json:"-"`
|
||||
// organizePlan is what organizing would do right now.
|
||||
type organizePlan struct {
|
||||
Scanned int
|
||||
Conforming int
|
||||
Moves int
|
||||
Skips map[string]int
|
||||
Items []organizeItem
|
||||
}
|
||||
|
||||
// sidecarExts move together with the song of the same name.
|
||||
@@ -277,9 +271,8 @@ func sidesOf(s string) []string {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (e *Engine) planOrganize(ctx context.Context, j *Job, l *libs, sides []string) (*OrganizePlan, error) {
|
||||
plan := &OrganizePlan{PairID: l.pair.ID, Sides: sides, Template: l.pair.OrganizeTemplate, CreatedAt: time.Now().Unix(), Skips: map[string]int{}}
|
||||
id := 0
|
||||
func (e *Engine) planOrganize(ctx context.Context, j *Job, l *libs, sides []string) (*organizePlan, error) {
|
||||
plan := &organizePlan{Skips: map[string]int{}}
|
||||
for _, side := range sides {
|
||||
root, _ := sideRoot(l, side)
|
||||
files := map[string]fsops.Info{}
|
||||
@@ -295,7 +288,7 @@ func (e *Engine) planOrganize(ctx context.Context, j *Job, l *libs, sides []stri
|
||||
j.tick()
|
||||
plan.Scanned++
|
||||
in := files[rel]
|
||||
item := OrganizeItem{Side: side, From: rel, ino: in.Ino, size: in.Size, mtime: in.MtimeNs}
|
||||
item := organizeItem{Side: side, From: rel, ino: in.Ino, size: in.Size, mtime: in.MtimeNs}
|
||||
if _, ok := tags.FormatOf(rel); !ok {
|
||||
item.Reason = "file type without editable tags"
|
||||
} else if fields, err := e.tagCache.get(ctx, e.tagTools, filepath.Join(root.Path, rel), in); err != nil {
|
||||
@@ -319,8 +312,6 @@ func (e *Engine) planOrganize(ctx context.Context, j *Job, l *libs, sides []stri
|
||||
item.Sidecars = sidecarsOf(root, rel)
|
||||
}
|
||||
}
|
||||
id++
|
||||
item.ID = id
|
||||
if item.Move {
|
||||
plan.Moves++
|
||||
} else {
|
||||
@@ -361,112 +352,13 @@ func sidecarsOf(root fsops.Root, rel string) []string {
|
||||
return out
|
||||
}
|
||||
|
||||
// StartOrganizePlan previews organizing one or both libraries.
|
||||
func (e *Engine) StartOrganizePlan(pairID int64, side string) (*Job, error) {
|
||||
if !e.TagEditing() {
|
||||
return nil, ErrNoTagTools
|
||||
}
|
||||
sides := sidesOf(side)
|
||||
if sides == nil {
|
||||
return nil, fmt.Errorf("%w: side must be slave, master or both", ErrInvalidBulk)
|
||||
}
|
||||
return e.startJob(pairID, "organize-preview", func(ctx context.Context, j *Job) error {
|
||||
l, err := e.loadPair(ctx, pairID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
plan, err := e.planOrganize(ctx, j, l, sides)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
e.plansMu.Lock()
|
||||
if e.plans == nil {
|
||||
e.plans = map[int64]*OrganizePlan{}
|
||||
}
|
||||
e.plans[pairID] = plan
|
||||
e.plansMu.Unlock()
|
||||
j.update(func(j *Job) {
|
||||
j.Message = fmt.Sprintf("%d to rename, %d already organized, %d skipped", plan.Moves, plan.Conforming, len(plan.Items)-plan.Moves)
|
||||
})
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
// OrganizePlanFor returns the latest preview (items filtered by "move" or
|
||||
// "skip"), newest first.
|
||||
func (e *Engine) OrganizePlanFor(pairID int64, filter string, offset, limit int) (*OrganizePlan, []OrganizeItem, int) {
|
||||
e.plansMu.Lock()
|
||||
defer e.plansMu.Unlock()
|
||||
plan := e.plans[pairID]
|
||||
if plan == nil {
|
||||
return nil, nil, 0
|
||||
}
|
||||
var items []OrganizeItem
|
||||
for _, it := range plan.Items {
|
||||
if filter == "" || (filter == "move") == it.Move {
|
||||
items = append(items, it)
|
||||
}
|
||||
}
|
||||
total := len(items)
|
||||
if limit <= 0 || limit > 1000 {
|
||||
limit = 200
|
||||
}
|
||||
if offset > total {
|
||||
offset = total
|
||||
}
|
||||
end := min(offset+limit, total)
|
||||
out := append([]OrganizeItem{}, items[offset:end]...)
|
||||
return plan, out, total
|
||||
}
|
||||
|
||||
// StartOrganizeApply performs the moves of the latest preview (all, or only
|
||||
// the given ids), then rescans. Each move is re-checked first.
|
||||
func (e *Engine) StartOrganizeApply(pairID int64, ids []int) (*Job, error) {
|
||||
e.plansMu.Lock()
|
||||
plan := e.plans[pairID]
|
||||
e.plansMu.Unlock()
|
||||
if plan == nil {
|
||||
return nil, fmt.Errorf("%w: preview the changes first", ErrInvalidBulk)
|
||||
}
|
||||
want := map[int]bool{}
|
||||
for _, id := range ids {
|
||||
want[id] = true
|
||||
}
|
||||
var items []OrganizeItem
|
||||
for _, it := range plan.Items {
|
||||
if it.Move && (len(want) == 0 || want[it.ID]) {
|
||||
items = append(items, it)
|
||||
}
|
||||
}
|
||||
if len(items) == 0 {
|
||||
return nil, fmt.Errorf("%w: nothing to rename", ErrInvalidBulk)
|
||||
}
|
||||
return e.startJob(pairID, "organize", func(ctx context.Context, j *Job) error {
|
||||
l, err := e.loadPair(ctx, pairID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := e.applyOrganize(ctx, j, l, items, "organize"); err != nil {
|
||||
return err
|
||||
}
|
||||
e.plansMu.Lock()
|
||||
delete(e.plans, pairID) // paths changed; the preview is stale
|
||||
e.plansMu.Unlock()
|
||||
if e.st.DryRun(ctx) {
|
||||
j.update(func(j *Job) { j.Message = "Dry run: renames recorded as planned in Activity" })
|
||||
return nil
|
||||
}
|
||||
return e.scan(ctx, j, pairID, "organize")
|
||||
})
|
||||
}
|
||||
|
||||
type moveDetail struct {
|
||||
Side string `json:"side"`
|
||||
Sidecars [][2]string `json:"sidecars,omitempty"` // [from, to]
|
||||
Art bool `json:"art,omitempty"` // a folder art file following its album
|
||||
}
|
||||
|
||||
func (e *Engine) applyOrganize(ctx context.Context, j *Job, l *libs, items []OrganizeItem, trigger string) error {
|
||||
func (e *Engine) applyOrganize(ctx context.Context, j *Job, l *libs, items []organizeItem, trigger string) error {
|
||||
j.phase("Renaming files", int64(len(items)))
|
||||
// Remember where each source folder's songs went, to move cover art along.
|
||||
type dirMove struct {
|
||||
@@ -527,7 +419,7 @@ func (e *Engine) moveFolderArt(ctx context.Context, l *libs, root fsops.Root, si
|
||||
if !en.Type().IsRegular() || !folderArt[strings.ToLower(en.Name())] {
|
||||
continue
|
||||
}
|
||||
item := OrganizeItem{Side: side, From: filepath.Join(from, en.Name()), To: filepath.Join(to, en.Name())}
|
||||
item := organizeItem{Side: side, From: filepath.Join(from, en.Name()), To: filepath.Join(to, en.Name())}
|
||||
e.mu.Lock()
|
||||
dst, err := root.Resolve(item.To)
|
||||
if err == nil {
|
||||
@@ -552,7 +444,7 @@ func setSidePath(o *store.Operation, side, rel string) {
|
||||
}
|
||||
}
|
||||
|
||||
func (e *Engine) moveOne(ctx context.Context, l *libs, it OrganizeItem, trigger string) error {
|
||||
func (e *Engine) moveOne(ctx context.Context, l *libs, it organizeItem, trigger string) error {
|
||||
e.mu.Lock()
|
||||
defer e.mu.Unlock()
|
||||
root, err := sideRoot(l, it.Side)
|
||||
@@ -715,7 +607,7 @@ func (e *Engine) autoOrganize(ctx context.Context, j *Job, l *libs) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
var moves []OrganizeItem
|
||||
var moves []organizeItem
|
||||
for _, it := range plan.Items {
|
||||
if it.Move {
|
||||
moves = append(moves, it)
|
||||
|
||||
@@ -60,14 +60,34 @@ func mkTagged(t *testing.T, path string, kv ...string) {
|
||||
}
|
||||
}
|
||||
|
||||
func (ev *env) organize(t *testing.T, side string) (*OrganizePlan, []OrganizeItem) {
|
||||
// plan and apply run the same code as automatic organizing.
|
||||
func (ev *env) plan(t *testing.T, sides ...string) *organizePlan {
|
||||
t.Helper()
|
||||
if _, err := ev.e.StartOrganizePlan(ev.pair.ID, side); err != nil {
|
||||
l, err := ev.e.loadPair(ctx, ev.pair.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ev.wait()
|
||||
plan, items, _ := ev.e.OrganizePlanFor(ev.pair.ID, "", 0, 1000)
|
||||
return plan, items
|
||||
p, err := ev.e.planOrganize(ctx, &Job{e: ev.e, Results: map[string]int{}}, l, sides)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return p
|
||||
}
|
||||
|
||||
func (ev *env) apply(t *testing.T, p *organizePlan) *Job {
|
||||
t.Helper()
|
||||
l, _ := ev.e.loadPair(ctx, ev.pair.ID)
|
||||
j := &Job{e: ev.e, Results: map[string]int{}}
|
||||
var moves []organizeItem
|
||||
for _, it := range p.Items {
|
||||
if it.Move {
|
||||
moves = append(moves, it)
|
||||
}
|
||||
}
|
||||
if err := ev.e.applyOrganize(ctx, j, l, moves, "test"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return j
|
||||
}
|
||||
|
||||
func TestOrganizeSlaveLibrary(t *testing.T) {
|
||||
@@ -84,17 +104,14 @@ func TestOrganizeSlaveLibrary(t *testing.T) {
|
||||
mkTagged(t, filepath.Join(s, "dupes/a.mp3"), "title", "Same", "artist", "X", "album", "Y", "track", "1")
|
||||
mkTagged(t, filepath.Join(s, "dupes/b.mp3"), "title", "Same", "artist", "X", "album", "Y", "track", "1")
|
||||
|
||||
plan, items := ev.organize(t, SideSlave)
|
||||
plan := ev.plan(t, SideSlave)
|
||||
if plan.Conforming != 1 || plan.Moves != 2 {
|
||||
t.Fatalf("want 1 conforming and 2 moves, got %+v %+v", plan, items)
|
||||
t.Fatalf("want 1 conforming and 2 moves, got %+v", plan)
|
||||
}
|
||||
if plan.Skips["missing artist or album artist tag"] != 1 || len(plan.Skips) != 2 {
|
||||
t.Fatalf("unexpected skips %+v", plan.Skips)
|
||||
}
|
||||
if _, err := ev.e.StartOrganizeApply(ev.pair.ID, nil); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
j := ev.wait()
|
||||
j := ev.apply(t, plan)
|
||||
if j.Results["renamed"] != 2 {
|
||||
t.Fatalf("apply: %+v %v", j.Results, j.Skipped)
|
||||
}
|
||||
@@ -139,11 +156,8 @@ func TestOrganizeKeepsHardlinks(t *testing.T) {
|
||||
if got := ev.match("Chris Picks/Nightmare.flac"); got.Status != store.StatusLinked || got.Kind != store.KindInode {
|
||||
t.Fatalf("a hardlink at another path should be recognised, got %s/%s", got.Status, got.Kind)
|
||||
}
|
||||
ev.organize(t, SideSlave)
|
||||
if _, err := ev.e.StartOrganizeApply(ev.pair.ID, nil); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ev.wait()
|
||||
ev.apply(t, ev.plan(t, SideSlave))
|
||||
ev.scan()
|
||||
moved := filepath.Join(ev.slave, "Avenged Sevenfold/Nightmare/01 Nightmare.flac")
|
||||
if !sameInode(t, m, moved) {
|
||||
t.Fatal("the moved slave file must still be the master's file")
|
||||
@@ -157,11 +171,7 @@ func TestOrganizeDryRunAndChangedFile(t *testing.T) {
|
||||
ev := tagEnv(t, true)
|
||||
p := filepath.Join(ev.slave, "x/song.mp3")
|
||||
mkTagged(t, p, "title", "Song", "artist", "A", "album", "B", "track", "1")
|
||||
ev.organize(t, SideSlave)
|
||||
if _, err := ev.e.StartOrganizeApply(ev.pair.ID, nil); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if j := ev.wait(); j.Results["renamed"] != 1 {
|
||||
if j := ev.apply(t, ev.plan(t, SideSlave)); j.Results["renamed"] != 1 {
|
||||
t.Fatalf("dry run should plan the rename: %+v", j.Results)
|
||||
}
|
||||
if _, err := os.Stat(p); err != nil {
|
||||
@@ -169,11 +179,10 @@ func TestOrganizeDryRunAndChangedFile(t *testing.T) {
|
||||
}
|
||||
|
||||
ev.st.SetSetting(ctx, "dry_run", "0")
|
||||
ev.organize(t, SideSlave)
|
||||
plan := ev.plan(t, SideSlave)
|
||||
mkTagged(t, p, "title", "Song", "artist", "A", "album", "B", "track", "1", "genre", "changed")
|
||||
ev.e.StartOrganizeApply(ev.pair.ID, nil)
|
||||
if j := ev.wait(); j.Results["renamed"] != 0 || j.Results["skipped"] != 1 {
|
||||
t.Fatalf("a file changed after the preview must be skipped: %+v", j.Results)
|
||||
if j := ev.apply(t, plan); j.Results["renamed"] != 0 || j.Results["skipped"] != 1 {
|
||||
t.Fatalf("a file changed after planning must be skipped: %+v", j.Results)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user