From ff7649600ba2f440f3140e8db073bbb52b43d27b Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Tue, 24 Feb 2026 16:11:41 -0500 Subject: [PATCH] refactored full rescan to use hook/lifecycle pattern and added rescan package --- .opencode/plans/refactoring-catalog.md | 18 +++------ backend/app.go | 14 +++---- backend/library/library.go | 48 +++++++++++------------- backend/library/rescan.go | 22 +++++------ frontend/wailsjs/go/library/Library.d.ts | 4 +- frontend/wailsjs/go/library/Library.js | 8 +--- frontend/wailsjs/go/models.ts | 12 ++++++ 7 files changed, 57 insertions(+), 69 deletions(-) diff --git a/.opencode/plans/refactoring-catalog.md b/.opencode/plans/refactoring-catalog.md index aa6e6f0..3b4d681 100644 --- a/.opencode/plans/refactoring-catalog.md +++ b/.opencode/plans/refactoring-catalog.md @@ -1,6 +1,6 @@ # Refactoring Catalog -Prioritized list of architectural improvements identified during a full codebase audit (Feb 2026). Items are grouped by priority — tackle P1 before adding major new features, P2 as convenient, P3 opportunistically. +Prioritized list of architectural improvements identified during a full codebase audit (Feb 2026). Items are grouped by priority — tackle P1 before adding major features, P2 as convenient, P3 opportunistically. --- @@ -50,23 +50,15 @@ Prioritized list of architectural improvements identified during a full codebase --- -### 10. Move `FullRescan` orchestration from library to app +### ~~10. Move `FullRescan` orchestration from library to app~~ — solved -**Problem:** `library.Library` holds references to the queue (`queueClearer` interface) and playlist service (`playlistRestorer` interface), set via `SetQueue()` and `SetPlaylistRestorer()`. The `FullRescan` method in `rescan.go` orchestrates clearing the queue and restoring playlists — cross-cutting concerns that aren't really library responsibilities. - -**Why it matters:** The library package shouldn't know about queue clearing or playlist restoration. This creates a dependency web (`app` -> `library` -> `queue`, `app` -> `library` -> `playlist`). - -**Approach:** Move the `FullRescan` orchestration to the `app` level. The app already has references to all three packages. The library would only expose `Scan()` and a `ClearAndRescan()` that handles only library concerns (clear DB, walk files, extract metadata). The app's `FullRescan` handler would call `queue.Clear()`, `library.ClearAndRescan()`, then `playlist.RestoreAll()`. +Replaced `queueClearer`/`playlistRestorer` interfaces and `SetQueue`/`SetPlaylistRestorer` setters with a single `RescanHooks` struct containing `PreClear`/`PostScan` function callbacks. The app wires `queue.Clear` and `playlist.RestoreAllPlaylists` as hooks, so the library no longer has any knowledge of or dependency on those packages. --- -### 11. Fix double `LibraryScanStarted` event during FullRescan +### ~~11. Fix double `LibraryScanStarted` event during FullRescan~~ — solved -**Problem:** `rescan.go:22` emits `LibraryScanStarted`, then calls `Scan()` which emits `LibraryScanStarted` again at `library.go:191`. The frontend receives two `LibraryScanStarted` events for a single full rescan. - -**Why it matters:** Frontend components may show duplicate "scanning" UI state transitions or start/reset loading indicators twice. - -**Approach:** Remove the `LibraryScanStarted` emission from either `FullRescan` or `Scan`. Since `Scan` is also called independently, keep it in `Scan` and remove it from `FullRescan`. +Removed the `LibraryScanStarted` emission from `FullRescan` (resolved as part of item #10). The event is now only emitted from `Scan()`, giving exactly one emission per rescan. --- diff --git a/backend/app.go b/backend/app.go index 51f778f..283c724 100644 --- a/backend/app.go +++ b/backend/app.go @@ -148,13 +148,13 @@ func (yj *YellowJacketApp) OnStartup(ctx context.Context) { yj.queue.SetPlayer(yj.player) yj.queue.RestoreState() - // Give the library a reference to the queue so FullRescan can - // clear the queue and stop playback before wiping data. - yj.library.SetQueue(yj.queue) - - // Give the library a reference to the playlist service so - // FullRescan can restore playlists from M3U8 files. - yj.library.SetPlaylistRestorer(yj.playlist) + // Wire cross-cutting rescan hooks so the library can + // orchestrate queue clearing and playlist restoration + // without depending on those packages directly. + yj.library.SetRescanHooks(library.RescanHooks{ + PreClear: yj.queue.Clear, + PostScan: yj.playlist.RestoreAllPlaylists, + }) // Register playback finished handler to drive queue auto-advance. yj.player.SetPlaybackFinishedHandler(yj.queue.OnPlaybackFinished) diff --git a/backend/library/library.go b/backend/library/library.go index 7d728b1..c9c7961 100644 --- a/backend/library/library.go +++ b/backend/library/library.go @@ -60,39 +60,33 @@ func newEntityCache() *entityCache { } } -// queueClearer is a narrow interface for clearing the playback queue. -type queueClearer interface { - Clear() -} - -// playlistRestorer is a narrow interface for restoring playlists -// from M3U8 files after a library rescan. -type playlistRestorer interface { - RestoreAllPlaylists() +// RescanHooks holds optional callbacks that run before and after +// the library-clear-and-scan phase of a full rescan. The app +// layer sets these to coordinate cross-cutting concerns (e.g. +// clearing the queue, restoring playlists) without the library +// needing to know about those packages. +type RescanHooks struct { + // PreClear runs before library data is wiped + // (e.g. clear queue and stop playback). + PreClear func() + // PostScan runs after the scan completes + // (e.g. restore playlists from M3U8 files). + PostScan func() } // Library manages scanning and querying the music collection. type Library struct { - ctx context.Context - logger *slog.Logger - conf *Config - db *database.DB - queue queueClearer - playlistRestorer playlistRestorer + ctx context.Context + logger *slog.Logger + conf *Config + db *database.DB + rescanHooks RescanHooks } -// SetQueue provides the library with a reference to the queue so -// that destructive operations like FullRescan can clear the queue -// and stop playback before wiping data. -func (l *Library) SetQueue(q queueClearer) { - l.queue = q -} - -// SetPlaylistRestorer provides the library with a reference to -// the playlist service so that FullRescan can restore playlists -// from M3U8 files after wiping data. -func (l *Library) SetPlaylistRestorer(p playlistRestorer) { - l.playlistRestorer = p +// SetRescanHooks provides optional hooks for cross-cutting +// orchestration during FullRescan. +func (l *Library) SetRescanHooks(h RescanHooks) { + l.rescanHooks = h } // NewLibrary creates a new library with the given configuration. diff --git a/backend/library/rescan.go b/backend/library/rescan.go index 8cd90ed..acf0d10 100644 --- a/backend/library/rescan.go +++ b/backend/library/rescan.go @@ -6,9 +6,6 @@ import ( "path/filepath" "time" - "github.com/wailsapp/wails/v2/pkg/runtime" - - "yellowjacket/backend/events" "yellowjacket/backend/system" ) @@ -19,14 +16,13 @@ import ( func (l *Library) FullRescan() (*ScanMetrics, error) { l.logger.Info("beginning full library rescan") - runtime.EventsEmit(l.ctx, events.LibraryScanStarted) - - // Stop playback and clear the queue before wiping data so - // the player is not referencing now-deleted tracks. + // Run the pre-clear hook (e.g. clear queue / stop playback) + // before wiping data so the player is not referencing + // now-deleted tracks. clearQueueStart := time.Now() - if l.queue != nil { - l.queue.Clear() + if l.rescanHooks.PreClear != nil { + l.rescanHooks.PreClear() } clearQueueDur := time.Since(clearQueueStart) @@ -68,10 +64,10 @@ func (l *Library) FullRescan() (*ScanMetrics, error) { clearDBDur + clearFilesDur } - // Restore playlists from M3U8 files now that the library - // has been rescanned and audio_files are populated again. - if l.playlistRestorer != nil { - l.playlistRestorer.RestoreAllPlaylists() + // Run the post-scan hook (e.g. restore playlists from M3U8 + // files) now that audio_files are populated again. + if l.rescanHooks.PostScan != nil { + l.rescanHooks.PostScan() } return metrics, err diff --git a/frontend/wailsjs/go/library/Library.d.ts b/frontend/wailsjs/go/library/Library.d.ts index 8ae186b..2b1ef4f 100755 --- a/frontend/wailsjs/go/library/Library.d.ts +++ b/frontend/wailsjs/go/library/Library.d.ts @@ -19,6 +19,4 @@ export function Scan():Promise; export function SetContext(arg1:context.Context):Promise; -export function SetPlaylistRestorer(arg1:library.playlistRestorer):Promise; - -export function SetQueue(arg1:library.queueClearer):Promise; +export function SetRescanHooks(arg1:library.RescanHooks):Promise; diff --git a/frontend/wailsjs/go/library/Library.js b/frontend/wailsjs/go/library/Library.js index d809140..4c1e191 100755 --- a/frontend/wailsjs/go/library/Library.js +++ b/frontend/wailsjs/go/library/Library.js @@ -34,10 +34,6 @@ export function SetContext(arg1) { return window['go']['library']['Library']['SetContext'](arg1); } -export function SetPlaylistRestorer(arg1) { - return window['go']['library']['Library']['SetPlaylistRestorer'](arg1); -} - -export function SetQueue(arg1) { - return window['go']['library']['Library']['SetQueue'](arg1); +export function SetRescanHooks(arg1) { + return window['go']['library']['Library']['SetRescanHooks'](arg1); } diff --git a/frontend/wailsjs/go/models.ts b/frontend/wailsjs/go/models.ts index ec4b1ac..1048cf9 100755 --- a/frontend/wailsjs/go/models.ts +++ b/frontend/wailsjs/go/models.ts @@ -40,6 +40,18 @@ export namespace library { this.Name = source["Name"]; } } + export class RescanHooks { + + + static createFrom(source: any = {}) { + return new RescanHooks(source); + } + + constructor(source: any = {}) { + if ('string' === typeof source) source = JSON.parse(source); + + } + } export class ScanMetrics { total: number; loadExisting: number;