refactor: simplify and align review-fix code

- luaplugin: route invokeHook/invokeHookWithData through the callBounded helper
  and extract logHookErr, removing the duplicated context-timeout dance.
- history: build the save buffer in a strings.Builder + atomic WriteFile
  (matching radio favorites), dropping the bespoke errWriter and its
  per-field write syscalls.
- plex: clone cached playlist/track slices on return, consistent with the
  spotify/navidrome defensive copies.
This commit is contained in:
Bjarne Øverli
2026-05-29 08:40:36 +02:00
parent f84e5843e5
commit 00440f970d
3 changed files with 36 additions and 72 deletions
+5 -4
View File
@@ -3,6 +3,7 @@ package plex
import (
"context"
"fmt"
"slices"
"sync"
"cliamp/config"
@@ -48,7 +49,7 @@ func (p *Provider) Name() string { return "Plex" }
func (p *Provider) Playlists() ([]playlist.PlaylistInfo, error) {
p.mu.Lock()
if p.playlistCache != nil {
cached := p.playlistCache
cached := slices.Clone(p.playlistCache)
p.mu.Unlock()
return cached, nil
}
@@ -85,7 +86,7 @@ func (p *Provider) Playlists() ([]playlist.PlaylistInfo, error) {
p.playlistCache = lists
p.mu.Unlock()
return lists, nil
return slices.Clone(lists), nil
}
// Refresh clears cached playlist and track data so the next call re-fetches
@@ -106,7 +107,7 @@ func (p *Provider) Tracks(albumRatingKey string) ([]playlist.Track, error) {
if p.trackCache != nil {
if cached, ok := p.trackCache[albumRatingKey]; ok {
p.mu.Unlock()
return cached, nil
return slices.Clone(cached), nil
}
}
p.mu.Unlock()
@@ -125,7 +126,7 @@ func (p *Provider) Tracks(albumRatingKey string) ([]playlist.Track, error) {
p.trackCache[albumRatingKey] = tracks
p.mu.Unlock()
return tracks, nil
return slices.Clone(tracks), nil
}
// SearchTracks searches the Plex music library for tracks matching query.
+19 -40
View File
@@ -174,44 +174,23 @@ func (s *Store) saveLocked(entries []Entry) error {
if err := os.MkdirAll(filepath.Dir(s.path), 0o755); err != nil {
return err
}
tmp := s.path + ".tmp"
f, err := os.Create(tmp)
if err != nil {
return err
}
ew := &errWriter{w: f}
// Build the full content in memory (writes to a Builder can't fail), then
// write a temp file and rename so a partial/failed write can never truncate
// the existing history file.
var b strings.Builder
for i, e := range entries {
if i > 0 {
ew.printf("\n")
fmt.Fprintln(&b)
}
writeEntry(ew, e)
writeEntry(&b, e)
}
if ew.err != nil {
f.Close()
os.Remove(tmp)
return ew.err
}
if err := f.Close(); err != nil {
os.Remove(tmp)
tmp := s.path + ".tmp"
if err := os.WriteFile(tmp, []byte(b.String()), 0o644); err != nil {
return err
}
return os.Rename(tmp, s.path)
}
// errWriter records the first write error so a chain of formatted writes can be
// checked once at the end instead of after every call.
type errWriter struct {
w io.Writer
err error
}
func (ew *errWriter) printf(format string, a ...any) {
if ew.err != nil {
return
}
_, ew.err = fmt.Fprintf(ew.w, format, a...)
}
// mergeTrackMeta keeps any non-empty metadata from the previous entry when a
// replay supplies a sparser track (e.g. an ICY title-only update arriving
// after the original tags were captured).
@@ -240,28 +219,28 @@ func mergeTrackMeta(prev, cur playlist.Track) playlist.Track {
return cur
}
func writeEntry(ew *errWriter, e Entry) {
ew.printf("[[entry]]\n")
ew.printf("played_at = %q\n", e.PlayedAt.UTC().Format(time.RFC3339))
ew.printf("path = %q\n", e.Track.Path)
ew.printf("title = %q\n", e.Track.Title)
func writeEntry(w io.Writer, e Entry) {
fmt.Fprintf(w, "[[entry]]\n")
fmt.Fprintf(w, "played_at = %q\n", e.PlayedAt.UTC().Format(time.RFC3339))
fmt.Fprintf(w, "path = %q\n", e.Track.Path)
fmt.Fprintf(w, "title = %q\n", e.Track.Title)
if e.Track.Artist != "" {
ew.printf("artist = %q\n", e.Track.Artist)
fmt.Fprintf(w, "artist = %q\n", e.Track.Artist)
}
if e.Track.Album != "" {
ew.printf("album = %q\n", e.Track.Album)
fmt.Fprintf(w, "album = %q\n", e.Track.Album)
}
if e.Track.Genre != "" {
ew.printf("genre = %q\n", e.Track.Genre)
fmt.Fprintf(w, "genre = %q\n", e.Track.Genre)
}
if e.Track.Year != 0 {
ew.printf("year = %d\n", e.Track.Year)
fmt.Fprintf(w, "year = %d\n", e.Track.Year)
}
if e.Track.TrackNumber != 0 {
ew.printf("track_number = %d\n", e.Track.TrackNumber)
fmt.Fprintf(w, "track_number = %d\n", e.Track.TrackNumber)
}
if e.Track.DurationSecs != 0 {
ew.printf("duration_secs = %d\n", e.Track.DurationSecs)
fmt.Fprintf(w, "duration_secs = %d\n", e.Track.DurationSecs)
}
}
+12 -28
View File
@@ -45,6 +45,14 @@ func (p *Plugin) callBounded(nret int, fn *lua.LFunction, args ...lua.LValue) er
return p.L.CallByParam(lua.P{Fn: fn, NRet: nret, Protect: true}, args...)
}
// logHookErr records a callback error to stderr and the plugin log.
func (m *Manager) logHookErr(name, label string, err error) {
log.Printf("[lua:%s] %s error: %v", name, label, err)
if m.logger != nil {
m.logger.log(name, "error", "%s error: %v", label, err)
}
}
// invokeHook calls a plugin's Lua callback under the plugin's mutex with a
// bounded context. Logs any error to the plugin log. Used by every dispatch
// site that fires Lua from Go (events, key binds, command handlers).
@@ -52,20 +60,8 @@ func (m *Manager) invokeHook(h *luaHook, label string, args ...lua.LValue) {
h.plugin.mu.Lock()
defer h.plugin.mu.Unlock()
ctx, cancel := context.WithTimeout(context.Background(), hookTimeout)
defer cancel()
h.plugin.L.SetContext(ctx)
defer h.plugin.L.RemoveContext()
if err := h.plugin.L.CallByParam(lua.P{
Fn: h.fn,
NRet: 0,
Protect: true,
}, args...); err != nil {
log.Printf("[lua:%s] %s error: %v", h.plugin.Name, label, err)
if m.logger != nil {
m.logger.log(h.plugin.Name, "error", "%s error: %v", label, err)
}
if err := h.plugin.callBounded(0, h.fn, args...); err != nil {
m.logHookErr(h.plugin.Name, label, err)
}
}
@@ -115,21 +111,9 @@ func (m *Manager) invokeHookWithData(h *luaHook, label string, data map[string]a
h.plugin.mu.Lock()
defer h.plugin.mu.Unlock()
ctx, cancel := context.WithTimeout(context.Background(), hookTimeout)
defer cancel()
h.plugin.L.SetContext(ctx)
defer h.plugin.L.RemoveContext()
arg := dataToTable(h.plugin.L, data)
if err := h.plugin.L.CallByParam(lua.P{
Fn: h.fn,
NRet: 0,
Protect: true,
}, arg); err != nil {
log.Printf("[lua:%s] %s error: %v", h.plugin.Name, label, err)
if m.logger != nil {
m.logger.log(h.plugin.Name, "error", "%s error: %v", label, err)
}
if err := h.plugin.callBounded(0, h.fn, arg); err != nil {
m.logHookErr(h.plugin.Name, label, err)
}
}