From a227278932fd6b981cb0ab11c3a0e81101ffcabd Mon Sep 17 00:00:00 2001 From: Rodrigo Verdiani Date: Mon, 6 Jul 2026 22:29:17 -0300 Subject: [PATCH] fix: handle library conversion and sqlite locking --- internal/store/store.go | 17 +++++- internal/store/store_test.go | 73 ++++++++++++++++++++++++ internal/web/library.go | 14 +++++ internal/web/render.go | 2 + internal/web/render_test.go | 16 ++++++ internal/web/web_test.go | 44 ++++++++++++++ web/templates/partials/scan_results.html | 23 +++++--- 7 files changed, 180 insertions(+), 9 deletions(-) create mode 100644 internal/web/render_test.go diff --git a/internal/store/store.go b/internal/store/store.go index 6aa9c7f..9b5fe13 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "net/url" "os" "path/filepath" @@ -25,10 +26,11 @@ func New(dbPath string) (*Store, error) { return nil, fmt.Errorf("create db directory: %w", err) } - db, err := sql.Open("sqlite", dbPath) + db, err := sql.Open("sqlite", dsn(dbPath)) if err != nil { return nil, fmt.Errorf("open database: %w", err) } + db.SetMaxOpenConns(1) pragmas := []string{ "PRAGMA journal_mode=WAL", @@ -62,3 +64,16 @@ func (s *Store) Close() error { func (s *Store) DB() *sql.DB { return s.db } + +func dsn(dbPath string) string { + values := url.Values{} + values.Add("_pragma", "journal_mode(WAL)") + values.Add("_pragma", "busy_timeout(5000)") + values.Add("_pragma", "foreign_keys(ON)") + + return (&url.URL{ + Scheme: "file", + Opaque: dbPath, + RawQuery: values.Encode(), + }).String() +} diff --git a/internal/store/store_test.go b/internal/store/store_test.go index e9f0c49..2cdd33d 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -2,8 +2,10 @@ package store import ( "context" + "fmt" "os" "path/filepath" + "sync" "testing" "coda/internal/models" @@ -39,6 +41,20 @@ func TestNewIdempotent(t *testing.T) { st2.Close() } +func TestNewRelativePath(t *testing.T) { + t.Chdir(t.TempDir()) + + st, err := New(filepath.Join("nested", "test.db")) + if err != nil { + t.Fatalf("New relative path: %v", err) + } + st.Close() + + if _, err := os.Stat(filepath.Join("nested", "test.db")); err != nil { + t.Fatalf("stat relative db: %v", err) + } +} + func TestJobCreateAndGet(t *testing.T) { st := openTestDB(t) ctx := context.Background() @@ -315,6 +331,63 @@ func TestLogCount(t *testing.T) { } } +func TestConcurrentWritesDoNotReturnBusy(t *testing.T) { + st := openTestDB(t) + ctx := context.Background() + + var wg sync.WaitGroup + errs := make(chan error, 90) + + for i := 0; i < 30; i++ { + wg.Add(3) + + go func(i int) { + defer wg.Done() + job := &models.Job{ + Artist: fmt.Sprintf("Artist %d", i), + Album: fmt.Sprintf("Album %d", i), + Path: fmt.Sprintf("/music/%d", i), + Status: models.StatusReceived, + Source: models.SourceManual, + } + if err := st.Jobs.Create(ctx, job); err != nil { + errs <- err + return + } + if err := st.Jobs.UpdateStatus(ctx, job.ID, models.StatusProcessing); err != nil { + errs <- err + return + } + if err := st.Jobs.Finish(ctx, job.ID, models.StatusDone, int64(i), ""); err != nil { + errs <- err + } + }(i) + + go func(i int) { + defer wg.Done() + errs <- st.Logs.Insert(ctx, &models.LogEntry{ + JobID: fmt.Sprintf("job-%d", i), + Level: "INFO", + Message: fmt.Sprintf("message %d", i), + }) + }(i) + + go func(i int) { + defer wg.Done() + errs <- st.Config.Set(ctx, fmt.Sprintf("key-%d", i), fmt.Sprintf("value-%d", i)) + }(i) + } + + wg.Wait() + close(errs) + + for err := range errs { + if err != nil { + t.Fatalf("concurrent write: %v", err) + } + } +} + func TestConfigSetGetGetAllDelete(t *testing.T) { st := openTestDB(t) ctx := context.Background() diff --git a/internal/web/library.go b/internal/web/library.go index fa6ebe5..3da1b8c 100644 --- a/internal/web/library.go +++ b/internal/web/library.go @@ -77,8 +77,18 @@ func (h *Handler) LibraryConvert(w http.ResponseWriter, r *http.Request) { return } + seen := map[string]struct{}{} resolved := make([]string, 0, len(paths)) for _, relPath := range paths { + relPath = strings.TrimSpace(relPath) + if relPath == "" { + continue + } + if _, ok := seen[relPath]; ok { + continue + } + seen[relPath] = struct{}{} + absPath, err := h.scanner.Resolve(relPath) if err != nil { slog.Warn("library convert: invalid path", "path", relPath, "error", err) @@ -87,6 +97,10 @@ func (h *Handler) LibraryConvert(w http.ResponseWriter, r *http.Request) { } resolved = append(resolved, absPath) } + if len(resolved) == 0 { + http.Error(w, "no paths provided", http.StatusBadRequest) + return + } var jobIDs []string for _, absPath := range resolved { diff --git a/internal/web/render.go b/internal/web/render.go index 06c3fda..0cd86c5 100644 --- a/internal/web/render.go +++ b/internal/web/render.go @@ -71,6 +71,8 @@ func formatDuration(ms int64) string { func statusBadge(level string) template.HTML { switch level { + case "DEBUG": + return template.HTML(`` + level + ``) case "INFO", "done": return template.HTML(`` + level + ``) case "WARN", "processing": diff --git a/internal/web/render_test.go b/internal/web/render_test.go new file mode 100644 index 0000000..f12c831 --- /dev/null +++ b/internal/web/render_test.go @@ -0,0 +1,16 @@ +package web + +import ( + "strings" + "testing" +) + +func TestStatusBadgeDebug(t *testing.T) { + badge := string(statusBadge("DEBUG")) + if !strings.Contains(badge, `class="badge`) { + t.Fatalf("DEBUG badge missing badge class: %s", badge) + } + if !strings.Contains(badge, "DEBUG") { + t.Fatalf("DEBUG badge missing label: %s", badge) + } +} diff --git a/internal/web/web_test.go b/internal/web/web_test.go index ba80434..c17c90b 100644 --- a/internal/web/web_test.go +++ b/internal/web/web_test.go @@ -46,6 +46,15 @@ func (m *mockStore) Delete(_ context.Context, key string) error { return nil } +type recordingEnqueuer struct { + inputs []jobs.EnqueueInput +} + +func (r *recordingEnqueuer) Enqueue(_ context.Context, in jobs.EnqueueInput) (string, error) { + r.inputs = append(r.inputs, in) + return "job-id", nil +} + func setupRouter(t *testing.T) chi.Router { t.Helper() @@ -245,6 +254,41 @@ func TestLibraryConvertJSON(t *testing.T) { } } +func TestLibraryConvertFormIgnoresEmptyAndDuplicatePaths(t *testing.T) { + musicPath := t.TempDir() + albumDir := filepath.Join(musicPath, "Test Artist", "Test Album") + if err := os.MkdirAll(albumDir, 0o755); err != nil { + t.Fatalf("mkdir album: %v", err) + } + + enqueuer := &recordingEnqueuer{} + h := &Handler{ + scanner: scanner.New(&config.Config{MusicPath: musicPath}), + enqueuer: enqueuer, + } + + form := url.Values{} + form.Add("paths", "") + form.Add("paths", "Test Artist/Test Album") + form.Add("paths", "Test Artist/Test Album") + + req := httptest.NewRequest(http.MethodPost, "/api/library/convert", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + + h.LibraryConvert(rec, req) + + if rec.Code != http.StatusAccepted { + t.Fatalf("status = %d, want %d; body: %s", rec.Code, http.StatusAccepted, rec.Body.String()) + } + if len(enqueuer.inputs) != 1 { + t.Fatalf("enqueued jobs = %d, want 1", len(enqueuer.inputs)) + } + if enqueuer.inputs[0].Artist != "Test Artist" || enqueuer.inputs[0].Album != "Test Album" { + t.Fatalf("enqueued input = %+v", enqueuer.inputs[0]) + } +} + func TestLibraryConvertInvalidPath(t *testing.T) { router := setupRouter(t) diff --git a/web/templates/partials/scan_results.html b/web/templates/partials/scan_results.html index 6da7817..ab3c148 100644 --- a/web/templates/partials/scan_results.html +++ b/web/templates/partials/scan_results.html @@ -1,10 +1,9 @@
-
- + @@ -14,7 +13,7 @@ {{ range . }} - + @@ -30,10 +29,18 @@
Album Artist Path
{{ .Album }} {{ .Artist }} {{ .RelPath }}