mirror of
https://github.com/navidrome/navidrome.git
synced 2026-07-10 01:28:50 +00:00
f48943c058
* fix(plugins): discard buffered scrobbles when a plugin is removed Scrobbles are buffered in the DB per service, keyed by the plugin name. When a plugin was removed (deleted from the plugins folder and detected by the sync), its pending buffer entries were left behind forever: the drain goroutine is stopped on the next scrobbler refresh, so the rows were never retried nor discarded. Worse, if a plugin with the same name was installed later, the stale entries would be drained into it - potentially a completely unrelated plugin that just reuses the name. Add a Discard(service) method to ScrobbleBufferRepository and call it from removePluginFromDB, right after the plugin record is deleted. Disabling a plugin intentionally keeps its buffered scrobbles, consistent with the buffer's purpose of surviving temporary outages, and transient unload/reload cycles during config updates are unaffected since they never delete the plugin record. * fix(plugins): don't wipe builtin scrobbler queues on plugin removal Buffer entries are keyed by service name only, and removePluginFromDB runs for any removed plugin file, so removing a plugin named e.g. lastfm.ndp - regardless of its capability - would discard the builtin Last.fm retry queue. Skip the discard when the plugin name is owned by a registered builtin scrobbler, exposed via a new scrobbler.IsBuiltinScrobbler helper. Reported by Codex review on the PR. Also drop the testBroker usage from the new removePluginFromDB spec: it is defined in manager_test.go which is excluded on Windows, breaking the Windows test build. sendPluginRefreshEvent is nil-safe, so no broker is needed.
85 lines
2.9 KiB
Go
85 lines
2.9 KiB
Go
package plugins
|
|
|
|
import (
|
|
"context"
|
|
"path/filepath"
|
|
"time"
|
|
|
|
"github.com/navidrome/navidrome/core/scrobbler"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/tests"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
. "github.com/onsi/gomega"
|
|
)
|
|
|
|
var _ = Describe("removePluginFromDB", func() {
|
|
It("discards buffered scrobbles for the removed plugin", func() {
|
|
ctx := context.Background()
|
|
buffer := tests.CreateMockedScrobbleBufferRepo()
|
|
Expect(buffer.Enqueue("my-plugin", "user1", "track1", time.Now())).To(Succeed())
|
|
Expect(buffer.Enqueue("other-plugin", "user1", "track2", time.Now())).To(Succeed())
|
|
|
|
repo := tests.CreateMockPluginRepo()
|
|
plugin := model.Plugin{ID: "my-plugin", Enabled: false}
|
|
repo.SetData(model.Plugins{plugin})
|
|
|
|
// No broker: sendPluginRefreshEvent is nil-safe, and testBroker is
|
|
// defined in manager_test.go, which is excluded on Windows.
|
|
m := &Manager{
|
|
ds: &tests.MockDataStore{MockedScrobbleBuffer: buffer},
|
|
}
|
|
Expect(m.removePluginFromDB(ctx, repo, &plugin)).To(Succeed())
|
|
|
|
_, err := repo.Get("my-plugin")
|
|
Expect(err).To(MatchError(model.ErrNotFound))
|
|
|
|
remaining, err := buffer.Length()
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(remaining).To(Equal(int64(1)))
|
|
entry, err := buffer.Next("other-plugin", "user1")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(entry).ToNot(BeNil(), "entries of other services must be kept")
|
|
})
|
|
|
|
It("keeps buffered scrobbles of a builtin scrobbler sharing the removed plugin's name", func() {
|
|
ctx := context.Background()
|
|
scrobbler.Register("builtin-svc", func(model.DataStore) scrobbler.Scrobbler { return nil })
|
|
buffer := tests.CreateMockedScrobbleBufferRepo()
|
|
Expect(buffer.Enqueue("builtin-svc", "user1", "track1", time.Now())).To(Succeed())
|
|
|
|
repo := tests.CreateMockPluginRepo()
|
|
plugin := model.Plugin{ID: "builtin-svc", Enabled: false}
|
|
repo.SetData(model.Plugins{plugin})
|
|
|
|
m := &Manager{
|
|
ds: &tests.MockDataStore{MockedScrobbleBuffer: buffer},
|
|
}
|
|
Expect(m.removePluginFromDB(ctx, repo, &plugin)).To(Succeed())
|
|
|
|
remaining, err := buffer.Length()
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(remaining).To(Equal(int64(1)), "builtin scrobbler queue must not be wiped")
|
|
})
|
|
})
|
|
|
|
var _ = Describe("ComputeFileSHA256", func() {
|
|
It("returns a consistent 64-char lowercase hex hash for the same file", func() {
|
|
dir := GinkgoT().TempDir()
|
|
ndpPath := filepath.Join(dir, "test.ndp")
|
|
err := createTestPackage(ndpPath, &Manifest{Name: "S", Author: "a", Version: "1.0.0"}, []byte{0x00, 0x61, 0x73, 0x6d})
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
hash1, err := ComputeFileSHA256(ndpPath)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
hash2, err := ComputeFileSHA256(ndpPath)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
Expect(hash1).To(Equal(hash2))
|
|
Expect(hash1).To(MatchRegexp(`^[0-9a-f]{64}$`))
|
|
})
|
|
|
|
It("returns an error for a non-existent path", func() {
|
|
_, err := ComputeFileSHA256(filepath.Join(GinkgoT().TempDir(), "does-not-exist.ndp"))
|
|
Expect(err).To(HaveOccurred())
|
|
})
|
|
})
|