diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 8938c0803..9488f20f7 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -157,6 +157,8 @@ jobs: exit 1 fi done + - run: ./.github/workflows/validate-translations.sh -v + check-push-enabled: name: Check Docker configuration diff --git a/.github/workflows/validate-translations.sh b/.github/workflows/validate-translations.sh new file mode 100755 index 000000000..a6b346e78 --- /dev/null +++ b/.github/workflows/validate-translations.sh @@ -0,0 +1,236 @@ +#!/bin/bash + +# validate-translations.sh +# +# This script validates the structure of JSON translation files by comparing them +# against the reference English translation file (ui/src/i18n/en.json). +# +# The script performs the following validations: +# 1. JSON syntax validation using jq +# 2. Structural validation - ensures all keys from English file are present +# 3. Reports missing keys (translation incomplete) +# 4. Reports extra keys (keys not in English reference, possibly deprecated) +# 5. Emits GitHub Actions annotations for CI/CD integration +# +# Usage: +# ./validate-translations.sh +# +# Environment Variables: +# EN_FILE - Path to reference English file (default: ui/src/i18n/en.json) +# TRANSLATION_DIR - Directory containing translation files (default: resources/i18n) +# +# Exit codes: +# 0 - All translations are valid +# 1 - One or more translations have structural issues +# +# GitHub Actions Integration: +# The script outputs GitHub Actions annotations using ::error and ::warning +# format that will be displayed in PR checks and workflow summaries. + +# Script to validate JSON translation files structure against en.json +set -e + +# Path to the reference English translation file +EN_FILE="${EN_FILE:-ui/src/i18n/en.json}" +TRANSLATION_DIR="${TRANSLATION_DIR:-resources/i18n}" +VERBOSE=false + +# Parse command line arguments +while [[ $# -gt 0 ]]; do + case "$1" in + -v|--verbose) + VERBOSE=true + shift + ;; + -h|--help) + echo "Usage: $0 [options]" + echo "" + echo "Validates JSON translation files structure against English reference file." + echo "" + echo "Options:" + echo " -h, --help Show this help message" + echo " -v, --verbose Show detailed output (default: only show errors)" + echo "" + echo "Environment Variables:" + echo " EN_FILE Path to reference English file (default: ui/src/i18n/en.json)" + echo " TRANSLATION_DIR Directory with translation files (default: resources/i18n)" + echo "" + echo "Examples:" + echo " $0 # Validate all translation files (quiet mode)" + echo " $0 -v # Validate with detailed output" + echo " EN_FILE=custom/en.json $0 # Use custom reference file" + echo " TRANSLATION_DIR=custom/i18n $0 # Use custom translations directory" + exit 0 + ;; + *) + echo "Unknown option: $1" >&2 + echo "Use --help for usage information" >&2 + exit 1 + ;; + esac +done + +# Color codes for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' # No Color + +if [[ "$VERBOSE" == "true" ]]; then + echo "Validating translation files structure against ${EN_FILE}..." +fi + +# Check if English reference file exists +if [[ ! -f "$EN_FILE" ]]; then + echo "::error::Reference file $EN_FILE not found" + exit 1 +fi + +# Function to extract all JSON keys from a file, creating a flat list of dot-separated paths +extract_keys() { + local file="$1" + jq -r 'paths(scalars) as $p | $p | join(".")' "$file" 2>/dev/null | sort +} + +# Function to extract all non-empty string keys (to identify structural issues) +extract_structure_keys() { + local file="$1" + # Get only keys where values are not empty strings + jq -r 'paths(scalars) as $p | select(getpath($p) != "") | $p | join(".")' "$file" 2>/dev/null | sort +} + +# Function to validate a single translation file +validate_translation() { + local translation_file="$1" + local filename=$(basename "$translation_file") + local has_errors=false + local verbose=${2:-false} + + if [[ "$verbose" == "true" ]]; then + echo "Validating $filename..." + fi + + # First validate JSON syntax + if ! jq empty "$translation_file" 2>/dev/null; then + echo "::error file=$translation_file::Invalid JSON syntax" + echo -e "${RED}✗ $filename has invalid JSON syntax${NC}" + return 1 + fi + + # Extract all keys from both files (for statistics) + local en_keys_file=$(mktemp) + local translation_keys_file=$(mktemp) + + extract_keys "$EN_FILE" > "$en_keys_file" + extract_keys "$translation_file" > "$translation_keys_file" + + # Extract only non-empty structure keys (to validate structural issues) + local en_structure_file=$(mktemp) + local translation_structure_file=$(mktemp) + + extract_structure_keys "$EN_FILE" > "$en_structure_file" + extract_structure_keys "$translation_file" > "$translation_structure_file" + + # Find structural issues: keys in translation not in English (misplaced) + local extra_keys=$(comm -13 "$en_keys_file" "$translation_keys_file") + + # Find missing keys (for statistics only) + local missing_keys=$(comm -23 "$en_keys_file" "$translation_keys_file") + + # Count keys for statistics + local total_en_keys=$(wc -l < "$en_keys_file") + local total_translation_keys=$(wc -l < "$translation_keys_file") + local missing_count=0 + local extra_count=0 + + if [[ -n "$missing_keys" ]]; then + missing_count=$(echo "$missing_keys" | grep -c '^' || echo 0) + fi + + if [[ -n "$extra_keys" ]]; then + extra_count=$(echo "$extra_keys" | grep -c '^' || echo 0) + has_errors=true + fi + + # Report extra/misplaced keys (these are structural issues) + if [[ -n "$extra_keys" ]]; then + if [[ "$verbose" == "true" ]]; then + echo -e "${YELLOW}Misplaced keys in $filename ($extra_count):${NC}" + fi + + while IFS= read -r key; do + # Try to find the line number + line=$(grep -n "\"$(echo "$key" | sed 's/.*\.//')" "$translation_file" | head -1 | cut -d: -f1) + line=${line:-1} # Default to line 1 if not found + + echo "::error file=$translation_file,line=$line::Misplaced key: $key" + + if [[ "$verbose" == "true" ]]; then + echo " + $key (line ~$line)" + fi + done <<< "$extra_keys" + fi + + # Clean up temp files + rm -f "$en_keys_file" "$translation_keys_file" "$en_structure_file" "$translation_structure_file" + + # Print statistics + if [[ "$verbose" == "true" ]]; then + echo " Keys: $total_translation_keys/$total_en_keys (Missing: $missing_count, Extra/Misplaced: $extra_count)" + + if [[ "$has_errors" == "true" ]]; then + echo -e "${RED}✗ $filename has structural issues${NC}" + else + echo -e "${GREEN}✓ $filename structure is valid${NC}" + fi + elif [[ "$has_errors" == "true" ]]; then + echo -e "${RED}✗ $filename has structural issues (Extra/Misplaced: $extra_count)${NC}" + fi + + return $([[ "$has_errors" == "true" ]] && echo 1 || echo 0) +} + +# Main validation loop +validation_failed=false +total_files=0 +failed_files=0 +valid_files=0 + +for translation_file in "$TRANSLATION_DIR"/*.json; do + if [[ -f "$translation_file" ]]; then + total_files=$((total_files + 1)) + if ! validate_translation "$translation_file" "$VERBOSE"; then + validation_failed=true + failed_files=$((failed_files + 1)) + else + valid_files=$((valid_files + 1)) + fi + + if [[ "$VERBOSE" == "true" ]]; then + echo "" # Add spacing between files + fi + fi +done + +# Summary +if [[ "$VERBOSE" == "true" ]]; then + echo "=========================================" + echo "Translation Validation Summary:" + echo " Total files: $total_files" + echo " Valid files: $valid_files" + echo " Files with structural issues: $failed_files" + echo "=========================================" +fi + +if [[ "$validation_failed" == "true" ]]; then + if [[ "$VERBOSE" == "true" ]]; then + echo -e "${RED}Translation validation failed - $failed_files file(s) have structural issues${NC}" + else + echo -e "${RED}Translation validation failed - $failed_files/$total_files file(s) have structural issues${NC}" + fi + exit 1 +elif [[ "$VERBOSE" == "true" ]]; then + echo -e "${GREEN}All translation files are structurally valid${NC}" +fi + +exit 0 \ No newline at end of file diff --git a/Makefile b/Makefile index 95515c3b8..90b4012f7 100644 --- a/Makefile +++ b/Makefile @@ -41,14 +41,21 @@ test: ##@Development Run Go tests go test -tags netgo $(PKG) .PHONY: test -testrace: ##@Development Run Go tests with race detector - go test -tags netgo -race -shuffle=on ./... -.PHONY: test - -testall: testrace ##@Development Run Go and JS tests - @(cd ./ui && npm run test) +testall: test-race test-i18n test-js ##@Development Run Go and JS tests .PHONY: testall +test-race: ##@Development Run Go tests with race detector + go test -tags netgo -race -shuffle=on ./... +.PHONY: test-race + +test-js: ##@Development Run JS tests + @(cd ./ui && npm run test) +.PHONY: test-js + +test-i18n: ##@Development Validate all translations files + ./.github/workflows/validate-translations.sh +.PHONY: test-i18n + install-golangci-lint: ##@Development Install golangci-lint if not present @PATH=$$PATH:./bin which golangci-lint > /dev/null || (echo "Installing golangci-lint..." && curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s v2.1.6) .PHONY: install-golangci-lint diff --git a/adapters/taglib/end_to_end_test.go b/adapters/taglib/end_to_end_test.go index 0b5126542..e4d94bb24 100644 --- a/adapters/taglib/end_to_end_test.go +++ b/adapters/taglib/end_to_end_test.go @@ -79,22 +79,29 @@ var _ = Describe("Extractor", func() { var e *extractor + parseTestFile := func(path string) *model.MediaFile { + mds, err := e.Parse(path) + Expect(err).ToNot(HaveOccurred()) + + info, ok := mds[path] + Expect(ok).To(BeTrue()) + + fileInfo, err := os.Stat(path) + Expect(err).ToNot(HaveOccurred()) + info.FileInfo = testFileInfo{FileInfo: fileInfo} + + metadata := metadata.New(path, info) + mf := metadata.ToMediaFile(1, "folderID") + return &mf + } + BeforeEach(func() { e = &extractor{} }) Describe("ReplayGain", func() { DescribeTable("test replaygain end-to-end", func(file string, trackGain, trackPeak, albumGain, albumPeak *float64) { - path := "tests/fixtures/" + file - mds, err := e.Parse(path) - Expect(err).ToNot(HaveOccurred()) - - info := mds[path] - fileInfo, _ := os.Stat(path) - info.FileInfo = testFileInfo{FileInfo: fileInfo} - - metadata := metadata.New(path, info) - mf := metadata.ToMediaFile(1, "folderID") + mf := parseTestFile("tests/fixtures/" + file) Expect(mf.RGTrackGain).To(Equal(trackGain)) Expect(mf.RGTrackPeak).To(Equal(trackPeak)) @@ -106,18 +113,82 @@ var _ = Describe("Extractor", func() { ) }) + Describe("lyrics", func() { + makeLyrics := func(code, secondLine string) model.Lyrics { + return model.Lyrics{ + DisplayArtist: "", + DisplayTitle: "", + Lang: code, + Line: []model.Line{ + {Start: gg.P(int64(0)), Value: "This is"}, + {Start: gg.P(int64(2500)), Value: secondLine}, + }, + Offset: nil, + Synced: true, + } + } + + It("should fetch both synced and unsynced lyrics in mixed flac", func() { + mf := parseTestFile("tests/fixtures/mixed-lyrics.flac") + + lyrics, err := mf.StructuredLyrics() + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics).To(HaveLen(2)) + + Expect(lyrics[0].Synced).To(BeTrue()) + Expect(lyrics[1].Synced).To(BeFalse()) + }) + + It("should handle mp3 with uslt and sylt", func() { + mf := parseTestFile("tests/fixtures/test.mp3") + + lyrics, err := mf.StructuredLyrics() + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics).To(HaveLen(4)) + + engSylt := makeLyrics("eng", "English SYLT") + engUslt := makeLyrics("eng", "English") + unsSylt := makeLyrics("xxx", "unspecified SYLT") + unsUslt := makeLyrics("xxx", "unspecified") + + // Why is the order inconsistent between runs? Nobody knows + Expect(lyrics).To(Or( + Equal(model.LyricList{engSylt, engUslt, unsSylt, unsUslt}), + Equal(model.LyricList{unsSylt, unsUslt, engSylt, engUslt}), + )) + }) + + DescribeTable("format-specific lyrics", func(file string, isId3 bool) { + mf := parseTestFile("tests/fixtures/" + file) + + lyrics, err := mf.StructuredLyrics() + Expect(err).To(Not(HaveOccurred())) + Expect(lyrics).To(HaveLen(2)) + + unspec := makeLyrics("xxx", "unspecified") + eng := makeLyrics("xxx", "English") + + if isId3 { + eng.Lang = "eng" + } + + Expect(lyrics).To(Or( + Equal(model.LyricList{unspec, eng}), + Equal(model.LyricList{eng, unspec}))) + }, + Entry("flac", "test.flac", false), + Entry("m4a", "test.m4a", false), + Entry("ogg", "test.ogg", false), + Entry("wma", "test.wma", false), + Entry("wv", "test.wv", false), + Entry("wav", "test.wav", true), + Entry("aiff", "test.aiff", true), + ) + }) + Describe("Participants", func() { DescribeTable("test tags consistent across formats", func(format string) { - path := "tests/fixtures/test." + format - mds, err := e.Parse(path) - Expect(err).ToNot(HaveOccurred()) - - info := mds[path] - fileInfo, _ := os.Stat(path) - info.FileInfo = testFileInfo{FileInfo: fileInfo} - - metadata := metadata.New(path, info) - mf := metadata.ToMediaFile(1, "folderID") + mf := parseTestFile("tests/fixtures/test." + format) for _, data := range roles { role := data.Role @@ -168,11 +239,40 @@ var _ = Describe("Extractor", func() { Entry("FLAC format", "flac"), Entry("M4a format", "m4a"), Entry("OGG format", "ogg"), - Entry("WMA format", "wv"), + Entry("WV format", "wv"), Entry("MP3 format", "mp3"), Entry("WAV format", "wav"), Entry("AIFF format", "aiff"), ) + + It("should parse wma", func() { + mf := parseTestFile("tests/fixtures/test.wma") + + for _, data := range roles { + role := data.Role + artists := data.ParticipantList + actual := mf.Participants[role] + + // WMA has no Arranger role + if role == model.RoleArranger { + Expect(actual).To(HaveLen(0)) + continue + } + + Expect(actual).To(HaveLen(len(artists)), role.String()) + + // For some bizarre reason, the order is inverted. We also don't get + // sort names or MBIDs + for i := range artists { + idx := len(artists) - 1 - i + + actualArtist := actual[i] + expectedArtist := artists[idx] + + Expect(actualArtist.Name).To(Equal(expectedArtist.Name)) + } + } + }) }) }) diff --git a/adapters/taglib/taglib_test.go b/adapters/taglib/taglib_test.go index 37b012763..f24c0e839 100644 --- a/adapters/taglib/taglib_test.go +++ b/adapters/taglib/taglib_test.go @@ -179,7 +179,7 @@ var _ = Describe("Extractor", func() { Entry("correctly parses wma/asf tags", "test.wma", "1.02s", 1, 44100, 16, "3.27 dB", "0.132914", "3.27 dB", "0.132914", false, true), // ffmpeg -f lavfi -i "sine=frequency=800:duration=1" test.wv - Entry("correctly parses wv (wavpak) tags", "test.wv", "1s", 1, 44100, 16, "3.43 dB", "0.125061", "3.43 dB", "0.125061", false, false), + Entry("correctly parses wv (wavpak) tags", "test.wv", "1s", 1, 44100, 16, "3.43 dB", "0.125061", "3.43 dB", "0.125061", false, true), // ffmpeg -f lavfi -i "sine=frequency=1000:duration=1" test.wav Entry("correctly parses wav tags", "test.wav", "1s", 1, 44100, 16, "3.06 dB", "0.125056", "3.06 dB", "0.125056", true, true), diff --git a/adapters/taglib/taglib_wrapper.cpp b/adapters/taglib/taglib_wrapper.cpp index 17c95bfc0..224642c6d 100644 --- a/adapters/taglib/taglib_wrapper.cpp +++ b/adapters/taglib/taglib_wrapper.cpp @@ -1,6 +1,5 @@ #include #include -#include #define TAGLIB_STATIC #include @@ -113,7 +112,7 @@ int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id) { strncpy(language, bv.data(), 3); } - char *val = (char *)frame->text().toCString(true); + char *val = const_cast(frame->text().toCString(true)); goPutLyrics(id, language, val); } @@ -132,7 +131,7 @@ int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id) { if (format == TagLib::ID3v2::SynchronizedLyricsFrame::AbsoluteMilliseconds) { for (const auto &line: frame->synchedText()) { - char *text = (char *)line.text.toCString(true); + char *text = const_cast(line.text.toCString(true)); goPutLyricLine(id, language, text, line.time); } } else if (format == TagLib::ID3v2::SynchronizedLyricsFrame::AbsoluteMpegFrames) { @@ -141,7 +140,7 @@ int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id) { if (sampleRate != 0) { for (const auto &line: frame->synchedText()) { const int timeInMs = (line.time * 1000) / sampleRate; - char *text = (char *)line.text.toCString(true); + char *text = const_cast(line.text.toCString(true)); goPutLyricLine(id, language, text, timeInMs); } } @@ -160,9 +159,9 @@ int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id) { if (m4afile != NULL) { const auto itemListMap = m4afile->tag()->itemMap(); for (const auto item: itemListMap) { - char *key = (char *)item.first.toCString(true); + char *key = const_cast(item.first.toCString(true)); for (const auto value: item.second.toStringList()) { - char *val = (char *)value.toCString(true); + char *val = const_cast(value.toCString(true)); goPutM4AStr(id, key, val); } } @@ -174,17 +173,24 @@ int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id) { const TagLib::ASF::Tag *asfTags{asfFile->tag()}; const auto itemListMap = asfTags->attributeListMap(); for (const auto item : itemListMap) { - tags.insert(item.first, item.second.front().toString()); + char *key = const_cast(item.first.toCString(true)); + + for (auto j = item.second.begin(); + j != item.second.end(); ++j) { + + char *val = const_cast(j->toString().toCString(true)); + goPutStr(id, key, val); + } } } // Send all collected tags to the Go map for (TagLib::PropertyMap::ConstIterator i = tags.begin(); i != tags.end(); ++i) { - char *key = (char *)i->first.toCString(true); + char *key = const_cast(i->first.toCString(true)); for (TagLib::StringList::ConstIterator j = i->second.begin(); j != i->second.end(); ++j) { - char *val = (char *)(*j).toCString(true); + char *val = const_cast((*j).toCString(true)); goPutStr(id, key, val); } } @@ -242,7 +248,19 @@ char has_cover(const TagLib::FileRef f) { // ----- WMA else if (TagLib::ASF::File * asfFile{dynamic_cast(f.file())}) { const TagLib::ASF::Tag *tag{ asfFile->tag() }; - hasCover = tag && asfFile->tag()->attributeListMap().contains("WM/Picture"); + hasCover = tag && tag->attributeListMap().contains("WM/Picture"); + } + // ----- DSF + else if (TagLib::DSF::File * dsffile{ dynamic_cast(f.file())}) { + const TagLib::ID3v2::Tag *tag { dsffile->tag() }; + hasCover = tag && !tag->frameListMap()["APIC"].isEmpty(); + } + // ----- WAVPAK (APE tag) + else if (TagLib::WavPack::File * wvFile{dynamic_cast(f.file())}) { + if (wvFile->hasAPETag()) { + // This is the particular string that Picard uses + hasCover = !wvFile->APETag()->itemListMap()["COVER ART (FRONT)"].isEmpty(); + } } return hasCover; diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index 59cf91e89..dc558c393 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -175,7 +175,7 @@ func GetPlaybackServer() playback.PlaybackServer { return playbackServer } -func getPluginManager() *plugins.Manager { +func getPluginManager() plugins.Manager { sqlDB := db.Db() dataStore := persistence.New(sqlDB) metricsMetrics := metrics.GetPrometheusInstance(dataStore) @@ -185,9 +185,9 @@ func getPluginManager() *plugins.Manager { // wire_injectors.go: -var allProviders = wire.NewSet(core.Set, artwork.Set, server.New, subsonic.New, nativeapi.New, public.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, scanner.New, scanner.NewWatcher, plugins.GetManager, metrics.GetPrometheusInstance, db.Db, wire.Bind(new(agents.PluginLoader), new(*plugins.Manager)), wire.Bind(new(scrobbler.PluginLoader), new(*plugins.Manager))) +var allProviders = wire.NewSet(core.Set, artwork.Set, server.New, subsonic.New, nativeapi.New, public.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, scanner.New, scanner.NewWatcher, plugins.GetManager, metrics.GetPrometheusInstance, db.Db, wire.Bind(new(agents.PluginLoader), new(plugins.Manager)), wire.Bind(new(scrobbler.PluginLoader), new(plugins.Manager))) -func GetPluginManager(ctx context.Context) *plugins.Manager { +func GetPluginManager(ctx context.Context) plugins.Manager { manager := getPluginManager() manager.SetSubsonicRouter(CreateSubsonicAPIRouter(ctx)) return manager diff --git a/cmd/wire_injectors.go b/cmd/wire_injectors.go index 9530e9bcf..e2bc6cd1b 100644 --- a/cmd/wire_injectors.go +++ b/cmd/wire_injectors.go @@ -42,8 +42,8 @@ var allProviders = wire.NewSet( plugins.GetManager, metrics.GetPrometheusInstance, db.Db, - wire.Bind(new(agents.PluginLoader), new(*plugins.Manager)), - wire.Bind(new(scrobbler.PluginLoader), new(*plugins.Manager)), + wire.Bind(new(agents.PluginLoader), new(plugins.Manager)), + wire.Bind(new(scrobbler.PluginLoader), new(plugins.Manager)), ) func CreateDataStore() model.DataStore { @@ -118,13 +118,13 @@ func GetPlaybackServer() playback.PlaybackServer { )) } -func getPluginManager() *plugins.Manager { +func getPluginManager() plugins.Manager { panic(wire.Build( allProviders, )) } -func GetPluginManager(ctx context.Context) *plugins.Manager { +func GetPluginManager(ctx context.Context) plugins.Manager { manager := getPluginManager() manager.SetSubsonicRouter(CreateSubsonicAPIRouter(ctx)) return manager diff --git a/conf/configuration.go b/conf/configuration.go index 258e3727f..bb1ae120b 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -264,13 +264,15 @@ func Load(noConfigDump bool) { os.Exit(1) } - if Server.Plugins.Folder == "" { - Server.Plugins.Folder = filepath.Join(Server.DataFolder, "plugins") - } - err = os.MkdirAll(Server.Plugins.Folder, 0700) - if err != nil { - _, _ = fmt.Fprintln(os.Stderr, "FATAL: Error creating plugins path:", err) - os.Exit(1) + if Server.Plugins.Enabled { + if Server.Plugins.Folder == "" { + Server.Plugins.Folder = filepath.Join(Server.DataFolder, "plugins") + } + err = os.MkdirAll(Server.Plugins.Folder, 0700) + if err != nil { + _, _ = fmt.Fprintln(os.Stderr, "FATAL: Error creating plugins path:", err) + os.Exit(1) + } } Server.ConfigFile = viper.GetViper().ConfigFileUsed() diff --git a/core/agents/agents.go b/core/agents/agents.go index efa9f383d..225411ecd 100644 --- a/core/agents/agents.go +++ b/core/agents/agents.go @@ -4,7 +4,6 @@ import ( "context" "slices" "strings" - "sync" "time" "github.com/navidrome/navidrome/conf" @@ -23,54 +22,9 @@ type PluginLoader interface { LoadMediaAgent(name string) (Interface, bool) } -type cachedAgent struct { - agent Interface - expiration time.Time -} - -// Encapsulates agent caching logic -// agentCache is a simple TTL cache for agents -// Not exported, only used by Agents - -type agentCache struct { - mu sync.Mutex - items map[string]cachedAgent - ttl time.Duration -} - -// TTL for cached agents -const agentCacheTTL = 5 * time.Minute - -func newAgentCache(ttl time.Duration) *agentCache { - return &agentCache{ - items: make(map[string]cachedAgent), - ttl: ttl, - } -} - -func (c *agentCache) Get(name string) Interface { - c.mu.Lock() - defer c.mu.Unlock() - cached, ok := c.items[name] - if ok && cached.expiration.After(time.Now()) { - return cached.agent - } - return nil -} - -func (c *agentCache) Set(name string, agent Interface) { - c.mu.Lock() - defer c.mu.Unlock() - c.items[name] = cachedAgent{ - agent: agent, - expiration: time.Now().Add(c.ttl), - } -} - type Agents struct { ds model.DataStore pluginLoader PluginLoader - cache *agentCache } // GetAgents returns the singleton instance of Agents @@ -85,18 +39,24 @@ func createAgents(ds model.DataStore, pluginLoader PluginLoader) *Agents { return &Agents{ ds: ds, pluginLoader: pluginLoader, - cache: newAgentCache(agentCacheTTL), } } -// getEnabledAgentNames returns the current list of enabled agent names, including: +// enabledAgent represents an enabled agent with its type information +type enabledAgent struct { + name string + isPlugin bool +} + +// getEnabledAgentNames returns the current list of enabled agents, including: // 1. Built-in agents and plugins from config (in the specified order) // 2. Always include LocalAgentName // 3. If config is empty, include ONLY LocalAgentName -func (a *Agents) getEnabledAgentNames() []string { +// Each enabledAgent contains the name and whether it's a plugin (true) or built-in (false) +func (a *Agents) getEnabledAgentNames() []enabledAgent { // If no agents configured, ONLY use the local agent if conf.Server.Agents == "" { - return []string{LocalAgentName} + return []enabledAgent{{name: LocalAgentName, isPlugin: false}} } // Get all available plugin names @@ -108,19 +68,13 @@ func (a *Agents) getEnabledAgentNames() []string { configuredAgents := strings.Split(conf.Server.Agents, ",") // Always add LocalAgentName if not already included - hasLocalAgent := false - for _, name := range configuredAgents { - if name == LocalAgentName { - hasLocalAgent = true - break - } - } + hasLocalAgent := slices.Contains(configuredAgents, LocalAgentName) if !hasLocalAgent { configuredAgents = append(configuredAgents, LocalAgentName) } // Filter to only include valid agents (built-in or plugins) - var validNames []string + var validAgents []enabledAgent for _, name := range configuredAgents { // Check if it's a built-in agent isBuiltIn := Map[name] != nil @@ -128,39 +82,35 @@ func (a *Agents) getEnabledAgentNames() []string { // Check if it's a plugin isPlugin := slices.Contains(availablePlugins, name) - if isBuiltIn || isPlugin { - validNames = append(validNames, name) + if isBuiltIn { + validAgents = append(validAgents, enabledAgent{name: name, isPlugin: false}) + } else if isPlugin { + validAgents = append(validAgents, enabledAgent{name: name, isPlugin: true}) } else { log.Warn("Unknown agent ignored", "name", name) } } - return validNames + return validAgents } -func (a *Agents) getAgent(name string) Interface { - // Check cache first - agent := a.cache.Get(name) - if agent != nil { - return agent - } - - // Try to get built-in agent - constructor, ok := Map[name] - if ok { - agent := constructor(a.ds) - if agent != nil { - a.cache.Set(name, agent) - return agent +func (a *Agents) getAgent(ea enabledAgent) Interface { + if ea.isPlugin { + // Try to load WASM plugin agent (if plugin loader is available) + if a.pluginLoader != nil { + agent, ok := a.pluginLoader.LoadMediaAgent(ea.name) + if ok && agent != nil { + return agent + } } - log.Debug("Built-in agent not available. Missing configuration?", "name", name) - } - - // Try to load WASM plugin agent (if plugin loader is available) - if a.pluginLoader != nil { - agent, ok := a.pluginLoader.LoadMediaAgent(name) - if ok && agent != nil { - a.cache.Set(name, agent) - return agent + } else { + // Try to get built-in agent + constructor, ok := Map[ea.name] + if ok { + agent := constructor(a.ds) + if agent != nil { + return agent + } + log.Debug("Built-in agent not available. Missing configuration?", "name", ea.name) } } @@ -179,8 +129,8 @@ func (a *Agents) GetArtistMBID(ctx context.Context, id string, name string) (str return "", nil } start := time.Now() - for _, agentName := range a.getEnabledAgentNames() { - ag := a.getAgent(agentName) + for _, enabledAgent := range a.getEnabledAgentNames() { + ag := a.getAgent(enabledAgent) if ag == nil { continue } @@ -208,8 +158,8 @@ func (a *Agents) GetArtistURL(ctx context.Context, id, name, mbid string) (strin return "", nil } start := time.Now() - for _, agentName := range a.getEnabledAgentNames() { - ag := a.getAgent(agentName) + for _, enabledAgent := range a.getEnabledAgentNames() { + ag := a.getAgent(enabledAgent) if ag == nil { continue } @@ -237,8 +187,8 @@ func (a *Agents) GetArtistBiography(ctx context.Context, id, name, mbid string) return "", nil } start := time.Now() - for _, agentName := range a.getEnabledAgentNames() { - ag := a.getAgent(agentName) + for _, enabledAgent := range a.getEnabledAgentNames() { + ag := a.getAgent(enabledAgent) if ag == nil { continue } @@ -271,8 +221,8 @@ func (a *Agents) GetSimilarArtists(ctx context.Context, id, name, mbid string, l overLimit := int(float64(limit) * conf.Server.DevExternalArtistFetchMultiplier) start := time.Now() - for _, agentName := range a.getEnabledAgentNames() { - ag := a.getAgent(agentName) + for _, enabledAgent := range a.getEnabledAgentNames() { + ag := a.getAgent(enabledAgent) if ag == nil { continue } @@ -304,8 +254,8 @@ func (a *Agents) GetArtistImages(ctx context.Context, id, name, mbid string) ([] return nil, nil } start := time.Now() - for _, agentName := range a.getEnabledAgentNames() { - ag := a.getAgent(agentName) + for _, enabledAgent := range a.getEnabledAgentNames() { + ag := a.getAgent(enabledAgent) if ag == nil { continue } @@ -338,8 +288,8 @@ func (a *Agents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid str overLimit := int(float64(count) * conf.Server.DevExternalArtistFetchMultiplier) start := time.Now() - for _, agentName := range a.getEnabledAgentNames() { - ag := a.getAgent(agentName) + for _, enabledAgent := range a.getEnabledAgentNames() { + ag := a.getAgent(enabledAgent) if ag == nil { continue } @@ -364,8 +314,8 @@ func (a *Agents) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (* return nil, ErrNotFound } start := time.Now() - for _, agentName := range a.getEnabledAgentNames() { - ag := a.getAgent(agentName) + for _, enabledAgent := range a.getEnabledAgentNames() { + ag := a.getAgent(enabledAgent) if ag == nil { continue } @@ -391,8 +341,8 @@ func (a *Agents) GetAlbumImages(ctx context.Context, name, artist, mbid string) return nil, ErrNotFound } start := time.Now() - for _, agentName := range a.getEnabledAgentNames() { - ag := a.getAgent(agentName) + for _, enabledAgent := range a.getEnabledAgentNames() { + ag := a.getAgent(enabledAgent) if ag == nil { continue } diff --git a/core/agents/agents_plugin_test.go b/core/agents/agents_plugin_test.go index 575fcbebe..b2791c00e 100644 --- a/core/agents/agents_plugin_test.go +++ b/core/agents/agents_plugin_test.go @@ -5,6 +5,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/slice" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -73,8 +74,10 @@ var _ = Describe("Agents with Plugin Loading", func() { mockLoader.pluginNames = append(mockLoader.pluginNames, "plugin_agent", "another_plugin") // Should only include the local agent - agentNames := agents.getEnabledAgentNames() - Expect(agentNames).To(HaveExactElements(LocalAgentName)) + enabledAgents := agents.getEnabledAgentNames() + Expect(enabledAgents).To(HaveLen(1)) + Expect(enabledAgents[0].name).To(Equal(LocalAgentName)) + Expect(enabledAgents[0].isPlugin).To(BeFalse()) // LocalAgent is built-in, not plugin }) It("should NOT include plugin agents when no config is specified", func() { @@ -85,9 +88,10 @@ var _ = Describe("Agents with Plugin Loading", func() { mockLoader.pluginNames = append(mockLoader.pluginNames, "plugin_agent") // Should only include the local agent - agentNames := agents.getEnabledAgentNames() - Expect(agentNames).To(HaveExactElements(LocalAgentName)) - Expect(agentNames).NotTo(ContainElement("plugin_agent")) + enabledAgents := agents.getEnabledAgentNames() + Expect(enabledAgents).To(HaveLen(1)) + Expect(enabledAgents[0].name).To(Equal(LocalAgentName)) + Expect(enabledAgents[0].isPlugin).To(BeFalse()) // LocalAgent is built-in, not plugin }) It("should include plugin agents in the enabled agents list ONLY when explicitly configured", func() { @@ -96,14 +100,24 @@ var _ = Describe("Agents with Plugin Loading", func() { // With no config, should not include plugin conf.Server.Agents = "" - agentNames := agents.getEnabledAgentNames() - Expect(agentNames).To(HaveExactElements(LocalAgentName)) - Expect(agentNames).NotTo(ContainElement("plugin_agent")) + enabledAgents := agents.getEnabledAgentNames() + Expect(enabledAgents).To(HaveLen(1)) + Expect(enabledAgents[0].name).To(Equal(LocalAgentName)) // When explicitly configured, should include plugin conf.Server.Agents = "plugin_agent" - agentNames = agents.getEnabledAgentNames() + enabledAgents = agents.getEnabledAgentNames() + var agentNames []string + var pluginAgentFound bool + for _, agent := range enabledAgents { + agentNames = append(agentNames, agent.name) + if agent.name == "plugin_agent" { + pluginAgentFound = true + Expect(agent.isPlugin).To(BeTrue()) // plugin_agent is a plugin + } + } Expect(agentNames).To(ContainElements(LocalAgentName, "plugin_agent")) + Expect(pluginAgentFound).To(BeTrue()) }) It("should only include configured plugin agents when config is specified", func() { @@ -114,9 +128,19 @@ var _ = Describe("Agents with Plugin Loading", func() { conf.Server.Agents = "plugin_one" // Verify only the configured one is included - agentNames := agents.getEnabledAgentNames() - Expect(agentNames).To(ContainElement("plugin_one")) + enabledAgents := agents.getEnabledAgentNames() + var agentNames []string + var pluginOneFound bool + for _, agent := range enabledAgents { + agentNames = append(agentNames, agent.name) + if agent.name == "plugin_one" { + pluginOneFound = true + Expect(agent.isPlugin).To(BeTrue()) // plugin_one is a plugin + } + } + Expect(agentNames).To(ContainElements(LocalAgentName, "plugin_one")) Expect(agentNames).NotTo(ContainElement("plugin_two")) + Expect(pluginOneFound).To(BeTrue()) }) It("should load plugin agents on demand", func() { @@ -140,31 +164,6 @@ var _ = Describe("Agents with Plugin Loading", func() { Expect(mockLoader.pluginCallCount["plugin_agent"]).To(Equal(1)) }) - It("should cache plugin agents", func() { - ctx := context.Background() - - // Configure to use our plugin - conf.Server.Agents = "plugin_agent" - - // Add a plugin agent - mockLoader.pluginNames = append(mockLoader.pluginNames, "plugin_agent") - mockLoader.loadedAgents["plugin_agent"] = &MockAgent{ - name: "plugin_agent", - mbid: "plugin-mbid", - } - - // Call multiple times - _, err := agents.GetArtistMBID(ctx, "123", "Artist") - Expect(err).ToNot(HaveOccurred()) - _, err = agents.GetArtistMBID(ctx, "123", "Artist") - Expect(err).ToNot(HaveOccurred()) - _, err = agents.GetArtistMBID(ctx, "123", "Artist") - Expect(err).ToNot(HaveOccurred()) - - // Should only load once - Expect(mockLoader.pluginCallCount["plugin_agent"]).To(Equal(1)) - }) - It("should try both built-in and plugin agents", func() { // Create a mock built-in agent Register("built_in", func(ds model.DataStore) Interface { @@ -188,8 +187,23 @@ var _ = Describe("Agents with Plugin Loading", func() { } // Verify that both are in the enabled list - agentNames := agents.getEnabledAgentNames() - Expect(agentNames).To(ContainElements("built_in", "plugin_agent")) + enabledAgents := agents.getEnabledAgentNames() + var agentNames []string + var builtInFound, pluginFound bool + for _, agent := range enabledAgents { + agentNames = append(agentNames, agent.name) + if agent.name == "built_in" { + builtInFound = true + Expect(agent.isPlugin).To(BeFalse()) // built-in agent + } + if agent.name == "plugin_agent" { + pluginFound = true + Expect(agent.isPlugin).To(BeTrue()) // plugin agent + } + } + Expect(agentNames).To(ContainElements("built_in", "plugin_agent", LocalAgentName)) + Expect(builtInFound).To(BeTrue()) + Expect(pluginFound).To(BeTrue()) }) It("should respect the order specified in configuration", func() { @@ -212,10 +226,56 @@ var _ = Describe("Agents with Plugin Loading", func() { conf.Server.Agents = "plugin_y,agent_b,plugin_x,agent_a" // Get the agent names - agentNames := agents.getEnabledAgentNames() + enabledAgents := agents.getEnabledAgentNames() + + // Extract just the names to verify the order + agentNames := slice.Map(enabledAgents, func(a enabledAgent) string { return a.name }) // Verify the order matches configuration, with LocalAgentName at the end Expect(agentNames).To(HaveExactElements("plugin_y", "agent_b", "plugin_x", "agent_a", LocalAgentName)) }) + + It("should NOT call LoadMediaAgent for built-in agents", func() { + ctx := context.Background() + + // Create a mock built-in agent + Register("builtin_agent", func(ds model.DataStore) Interface { + return &MockAgent{ + name: "builtin_agent", + mbid: "builtin-mbid", + } + }) + defer func() { + delete(Map, "builtin_agent") + }() + + // Configure to use only built-in agents + conf.Server.Agents = "builtin_agent" + + // Call GetArtistMBID which should only use the built-in agent + mbid, err := agents.GetArtistMBID(ctx, "123", "Artist") + + Expect(err).ToNot(HaveOccurred()) + Expect(mbid).To(Equal("builtin-mbid")) + + // Verify LoadMediaAgent was NEVER called (no plugin loading for built-in agents) + Expect(mockLoader.pluginCallCount).To(BeEmpty()) + }) + + It("should NOT call LoadMediaAgent for invalid agent names", func() { + ctx := context.Background() + + // Configure with an invalid agent name (not built-in, not a plugin) + conf.Server.Agents = "invalid_agent" + + // This should only result in using the local agent (as the invalid one is ignored) + _, err := agents.GetArtistMBID(ctx, "123", "Artist") + + // Should get ErrNotFound since only local agent is available and it returns not found for this operation + Expect(err).To(MatchError(ErrNotFound)) + + // Verify LoadMediaAgent was NEVER called for the invalid agent + Expect(mockLoader.pluginCallCount).To(BeEmpty()) + }) }) }) diff --git a/core/agents/agents_test.go b/core/agents/agents_test.go index 0732d43ef..0b7eec282 100644 --- a/core/agents/agents_test.go +++ b/core/agents/agents_test.go @@ -56,8 +56,8 @@ var _ = Describe("Agents", func() { It("does not register disabled agents", func() { var ags []string - for _, name := range ag.getEnabledAgentNames() { - agent := ag.getAgent(name) + for _, enabledAgent := range ag.getEnabledAgentNames() { + agent := ag.getAgent(enabledAgent) if agent != nil { ags = append(ags, agent.AgentName()) } diff --git a/core/playback/mpv/mpv_test.go b/core/playback/mpv/mpv_test.go index 08432bef3..20c02501b 100644 --- a/core/playback/mpv/mpv_test.go +++ b/core/playback/mpv/mpv_test.go @@ -372,7 +372,7 @@ goto loop ` } else { scriptExt = ".sh" - scriptContent = `#!/bin/bash + scriptContent = `#!/bin/sh echo "$0" for arg in "$@"; do echo "$arg" diff --git a/core/scrobbler/play_tracker.go b/core/scrobbler/play_tracker.go index 7ce9522b9..e4e052779 100644 --- a/core/scrobbler/play_tracker.go +++ b/core/scrobbler/play_tracker.go @@ -138,23 +138,18 @@ func (p *playTracker) refreshPluginScrobblers() { } } + type stoppableScrobbler interface { + Scrobbler + Stop() + } + // Process removals - remove plugins that no longer exist for name, scrobbler := range p.pluginScrobblers { if _, exists := current[name]; !exists { - // Type assertion to access the Stop method - // We need to ensure this works even with interface objects - if bs, ok := scrobbler.(*bufferedScrobbler); ok { - log.Debug("Stopping buffered scrobbler goroutine", "name", name) - bs.Stop() - } else { - // For tests - try to see if this is a mock with a Stop method - type stoppable interface { - Stop() - } - if s, ok := scrobbler.(stoppable); ok { - log.Debug("Stopping mock scrobbler", "name", name) - s.Stop() - } + // If the scrobbler implements stoppableScrobbler, call Stop() before removing it + if stoppable, ok := scrobbler.(stoppableScrobbler); ok { + log.Debug("Stopping scrobbler", "name", name) + stoppable.Stop() } delete(p.pluginScrobblers, name) } diff --git a/model/metadata/metadata.go b/model/metadata/metadata.go index aea4238a4..1372d0034 100644 --- a/model/metadata/metadata.go +++ b/model/metadata/metadata.go @@ -245,10 +245,14 @@ func processPairMapping(name model.TagName, mapping model.TagConf, lowered model } } + // always parse id3 pairs. For lyrics, Taglib appears to always provide lyrics:xxx + // Prefer that over format-specific tags + id3Base := parseID3Pairs(name, lowered) + if len(aliasValues) > 0 { - return parseVorbisPairs(aliasValues) + id3Base = append(id3Base, parseVorbisPairs(aliasValues)...) } - return parseID3Pairs(name, lowered) + return id3Base } func parseID3Pairs(name model.TagName, lowered model.Tags) []string { diff --git a/plugins/adapter_media_agent.go b/plugins/adapter_media_agent.go index 43fc0e030..eca891275 100644 --- a/plugins/adapter_media_agent.go +++ b/plugins/adapter_media_agent.go @@ -10,14 +10,14 @@ import ( ) // NewWasmMediaAgent creates a new adapter for a MetadataAgent plugin -func newWasmMediaAgent(wasmPath, pluginID string, m *Manager, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin { +func newWasmMediaAgent(wasmPath, pluginID string, m *managerImpl, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin { loader, err := api.NewMetadataAgentPlugin(context.Background(), api.WazeroRuntime(runtime), api.WazeroModuleConfig(mc)) if err != nil { log.Error("Error creating media metadata service plugin", "plugin", pluginID, "path", wasmPath, err) return nil } return &wasmMediaAgent{ - wasmBasePlugin: newWasmBasePlugin[api.MetadataAgent, *api.MetadataAgentPlugin]( + baseCapability: newBaseCapability[api.MetadataAgent, *api.MetadataAgentPlugin]( wasmPath, pluginID, CapabilityMetadataAgent, @@ -32,7 +32,7 @@ func newWasmMediaAgent(wasmPath, pluginID string, m *Manager, runtime api.Wazero // wasmMediaAgent adapts a MetadataAgent plugin to implement the agents.Interface type wasmMediaAgent struct { - *wasmBasePlugin[api.MetadataAgent, *api.MetadataAgentPlugin] + *baseCapability[api.MetadataAgent, *api.MetadataAgentPlugin] } func (w *wasmMediaAgent) AgentName() string { @@ -49,108 +49,108 @@ func (w *wasmMediaAgent) mapError(err error) error { // Album-related methods func (w *wasmMediaAgent) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*agents.AlbumInfo, error) { - return callMethod(ctx, w, "GetAlbumInfo", func(inst api.MetadataAgent) (*agents.AlbumInfo, error) { - res, err := inst.GetAlbumInfo(ctx, &api.AlbumInfoRequest{Name: name, Artist: artist, Mbid: mbid}) - if err != nil { - return nil, w.mapError(err) - } - if res == nil || res.Info == nil { - return nil, agents.ErrNotFound - } - info := res.Info - return &agents.AlbumInfo{ - Name: info.Name, - MBID: info.Mbid, - Description: info.Description, - URL: info.Url, - }, nil + res, err := callMethod(ctx, w, "GetAlbumInfo", func(inst api.MetadataAgent) (*api.AlbumInfoResponse, error) { + return inst.GetAlbumInfo(ctx, &api.AlbumInfoRequest{Name: name, Artist: artist, Mbid: mbid}) }) + if err != nil { + return nil, w.mapError(err) + } + if res == nil || res.Info == nil { + return nil, agents.ErrNotFound + } + info := res.Info + return &agents.AlbumInfo{ + Name: info.Name, + MBID: info.Mbid, + Description: info.Description, + URL: info.Url, + }, nil } func (w *wasmMediaAgent) GetAlbumImages(ctx context.Context, name, artist, mbid string) ([]agents.ExternalImage, error) { - return callMethod(ctx, w, "GetAlbumImages", func(inst api.MetadataAgent) ([]agents.ExternalImage, error) { - res, err := inst.GetAlbumImages(ctx, &api.AlbumImagesRequest{Name: name, Artist: artist, Mbid: mbid}) - if err != nil { - return nil, w.mapError(err) - } - return convertExternalImages(res.Images), nil + res, err := callMethod(ctx, w, "GetAlbumImages", func(inst api.MetadataAgent) (*api.AlbumImagesResponse, error) { + return inst.GetAlbumImages(ctx, &api.AlbumImagesRequest{Name: name, Artist: artist, Mbid: mbid}) }) + if err != nil { + return nil, w.mapError(err) + } + return convertExternalImages(res.Images), nil } // Artist-related methods func (w *wasmMediaAgent) GetArtistMBID(ctx context.Context, id string, name string) (string, error) { - return callMethod(ctx, w, "GetArtistMBID", func(inst api.MetadataAgent) (string, error) { - res, err := inst.GetArtistMBID(ctx, &api.ArtistMBIDRequest{Id: id, Name: name}) - if err != nil { - return "", w.mapError(err) - } - return res.GetMbid(), nil + res, err := callMethod(ctx, w, "GetArtistMBID", func(inst api.MetadataAgent) (*api.ArtistMBIDResponse, error) { + return inst.GetArtistMBID(ctx, &api.ArtistMBIDRequest{Id: id, Name: name}) }) + if err != nil { + return "", w.mapError(err) + } + return res.GetMbid(), nil } func (w *wasmMediaAgent) GetArtistURL(ctx context.Context, id, name, mbid string) (string, error) { - return callMethod(ctx, w, "GetArtistURL", func(inst api.MetadataAgent) (string, error) { - res, err := inst.GetArtistURL(ctx, &api.ArtistURLRequest{Id: id, Name: name, Mbid: mbid}) - if err != nil { - return "", w.mapError(err) - } - return res.GetUrl(), nil + res, err := callMethod(ctx, w, "GetArtistURL", func(inst api.MetadataAgent) (*api.ArtistURLResponse, error) { + return inst.GetArtistURL(ctx, &api.ArtistURLRequest{Id: id, Name: name, Mbid: mbid}) }) + if err != nil { + return "", w.mapError(err) + } + return res.GetUrl(), nil } func (w *wasmMediaAgent) GetArtistBiography(ctx context.Context, id, name, mbid string) (string, error) { - return callMethod(ctx, w, "GetArtistBiography", func(inst api.MetadataAgent) (string, error) { - res, err := inst.GetArtistBiography(ctx, &api.ArtistBiographyRequest{Id: id, Name: name, Mbid: mbid}) - if err != nil { - return "", w.mapError(err) - } - return res.GetBiography(), nil + res, err := callMethod(ctx, w, "GetArtistBiography", func(inst api.MetadataAgent) (*api.ArtistBiographyResponse, error) { + return inst.GetArtistBiography(ctx, &api.ArtistBiographyRequest{Id: id, Name: name, Mbid: mbid}) }) + if err != nil { + return "", w.mapError(err) + } + return res.GetBiography(), nil } func (w *wasmMediaAgent) GetSimilarArtists(ctx context.Context, id, name, mbid string, limit int) ([]agents.Artist, error) { - return callMethod(ctx, w, "GetSimilarArtists", func(inst api.MetadataAgent) ([]agents.Artist, error) { - resp, err := inst.GetSimilarArtists(ctx, &api.ArtistSimilarRequest{Id: id, Name: name, Mbid: mbid, Limit: int32(limit)}) - if err != nil { - return nil, w.mapError(err) - } - artists := make([]agents.Artist, 0, len(resp.GetArtists())) - for _, a := range resp.GetArtists() { - artists = append(artists, agents.Artist{ - Name: a.GetName(), - MBID: a.GetMbid(), - }) - } - return artists, nil + resp, err := callMethod(ctx, w, "GetSimilarArtists", func(inst api.MetadataAgent) (*api.ArtistSimilarResponse, error) { + return inst.GetSimilarArtists(ctx, &api.ArtistSimilarRequest{Id: id, Name: name, Mbid: mbid, Limit: int32(limit)}) }) + if err != nil { + return nil, w.mapError(err) + } + artists := make([]agents.Artist, 0, len(resp.GetArtists())) + for _, a := range resp.GetArtists() { + artists = append(artists, agents.Artist{ + Name: a.GetName(), + MBID: a.GetMbid(), + }) + } + return artists, nil } func (w *wasmMediaAgent) GetArtistImages(ctx context.Context, id, name, mbid string) ([]agents.ExternalImage, error) { - return callMethod(ctx, w, "GetArtistImages", func(inst api.MetadataAgent) ([]agents.ExternalImage, error) { - res, err := inst.GetArtistImages(ctx, &api.ArtistImageRequest{Id: id, Name: name, Mbid: mbid}) - if err != nil { - return nil, w.mapError(err) - } - return convertExternalImages(res.Images), nil + resp, err := callMethod(ctx, w, "GetArtistImages", func(inst api.MetadataAgent) (*api.ArtistImageResponse, error) { + return inst.GetArtistImages(ctx, &api.ArtistImageRequest{Id: id, Name: name, Mbid: mbid}) }) + if err != nil { + return nil, w.mapError(err) + } + return convertExternalImages(resp.Images), nil } func (w *wasmMediaAgent) GetArtistTopSongs(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) { - return callMethod(ctx, w, "GetArtistTopSongs", func(inst api.MetadataAgent) ([]agents.Song, error) { - resp, err := inst.GetArtistTopSongs(ctx, &api.ArtistTopSongsRequest{Id: id, ArtistName: artistName, Mbid: mbid, Count: int32(count)}) - if err != nil { - return nil, w.mapError(err) - } - songs := make([]agents.Song, 0, len(resp.GetSongs())) - for _, s := range resp.GetSongs() { - songs = append(songs, agents.Song{ - Name: s.GetName(), - MBID: s.GetMbid(), - }) - } - return songs, nil + resp, err := callMethod(ctx, w, "GetArtistTopSongs", func(inst api.MetadataAgent) (*api.ArtistTopSongsResponse, error) { + return inst.GetArtistTopSongs(ctx, &api.ArtistTopSongsRequest{Id: id, ArtistName: artistName, Mbid: mbid, Count: int32(count)}) }) + if err != nil { + return nil, w.mapError(err) + } + songs := make([]agents.Song, 0, len(resp.GetSongs())) + for _, s := range resp.GetSongs() { + songs = append(songs, agents.Song{ + Name: s.GetName(), + MBID: s.GetMbid(), + }) + } + return songs, nil } // Helper function to convert ExternalImage objects from the API to the agents package diff --git a/plugins/adapter_media_agent_test.go b/plugins/adapter_media_agent_test.go index 709fd62cd..f8b61ea5f 100644 --- a/plugins/adapter_media_agent_test.go +++ b/plugins/adapter_media_agent_test.go @@ -7,6 +7,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/plugins/api" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -14,7 +15,7 @@ import ( var _ = Describe("Adapter Media Agent", func() { var ctx context.Context - var mgr *Manager + var mgr *managerImpl BeforeEach(func() { ctx = GinkgoT().Context() @@ -23,7 +24,7 @@ var _ = Describe("Adapter Media Agent", func() { DeferCleanup(configtest.SetupConfig()) conf.Server.Plugins.Folder = testDataDir - mgr = createManager(nil, nil) + mgr = createManager(nil, metrics.NewNoopInstance()) mgr.ScanPlugins() }) diff --git a/plugins/adapter_scheduler_callback.go b/plugins/adapter_scheduler_callback.go index 2fe94d613..64b7eefff 100644 --- a/plugins/adapter_scheduler_callback.go +++ b/plugins/adapter_scheduler_callback.go @@ -9,14 +9,14 @@ import ( ) // newWasmSchedulerCallback creates a new adapter for a SchedulerCallback plugin -func newWasmSchedulerCallback(wasmPath, pluginID string, m *Manager, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin { +func newWasmSchedulerCallback(wasmPath, pluginID string, m *managerImpl, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin { loader, err := api.NewSchedulerCallbackPlugin(context.Background(), api.WazeroRuntime(runtime), api.WazeroModuleConfig(mc)) if err != nil { log.Error("Error creating scheduler callback plugin", "plugin", pluginID, "path", wasmPath, err) return nil } return &wasmSchedulerCallback{ - wasmBasePlugin: newWasmBasePlugin[api.SchedulerCallback, *api.SchedulerCallbackPlugin]( + baseCapability: newBaseCapability[api.SchedulerCallback, *api.SchedulerCallbackPlugin]( wasmPath, pluginID, CapabilitySchedulerCallback, @@ -31,5 +31,16 @@ func newWasmSchedulerCallback(wasmPath, pluginID string, m *Manager, runtime api // wasmSchedulerCallback adapts a SchedulerCallback plugin type wasmSchedulerCallback struct { - *wasmBasePlugin[api.SchedulerCallback, *api.SchedulerCallbackPlugin] + *baseCapability[api.SchedulerCallback, *api.SchedulerCallbackPlugin] +} + +func (w *wasmSchedulerCallback) OnSchedulerCallback(ctx context.Context, scheduleID string, payload []byte, isRecurring bool) error { + _, err := callMethod(ctx, w, "OnSchedulerCallback", func(inst api.SchedulerCallback) (*api.SchedulerCallbackResponse, error) { + return inst.OnSchedulerCallback(ctx, &api.SchedulerCallbackRequest{ + ScheduleId: scheduleID, + Payload: payload, + IsRecurring: isRecurring, + }) + }) + return err } diff --git a/plugins/adapter_scrobbler.go b/plugins/adapter_scrobbler.go index b9c27901f..54c6af127 100644 --- a/plugins/adapter_scrobbler.go +++ b/plugins/adapter_scrobbler.go @@ -12,14 +12,14 @@ import ( "github.com/tetratelabs/wazero" ) -func newWasmScrobblerPlugin(wasmPath, pluginID string, m *Manager, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin { +func newWasmScrobblerPlugin(wasmPath, pluginID string, m *managerImpl, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin { loader, err := api.NewScrobblerPlugin(context.Background(), api.WazeroRuntime(runtime), api.WazeroModuleConfig(mc)) if err != nil { log.Error("Error creating scrobbler service plugin", "plugin", pluginID, "path", wasmPath, err) return nil } return &wasmScrobblerPlugin{ - wasmBasePlugin: newWasmBasePlugin[api.Scrobbler, *api.ScrobblerPlugin]( + baseCapability: newBaseCapability[api.Scrobbler, *api.ScrobblerPlugin]( wasmPath, pluginID, CapabilityScrobbler, @@ -33,7 +33,7 @@ func newWasmScrobblerPlugin(wasmPath, pluginID string, m *Manager, runtime api.W } type wasmScrobblerPlugin struct { - *wasmBasePlugin[api.Scrobbler, *api.ScrobblerPlugin] + *baseCapability[api.Scrobbler, *api.ScrobblerPlugin] } func (w *wasmScrobblerPlugin) IsAuthorized(ctx context.Context, userId string) bool { @@ -44,21 +44,16 @@ func (w *wasmScrobblerPlugin) IsAuthorized(ctx context.Context, userId string) b username = u.UserName } } - - result, err := callMethod(ctx, w, "IsAuthorized", func(inst api.Scrobbler) (bool, error) { - resp, err := inst.IsAuthorized(ctx, &api.ScrobblerIsAuthorizedRequest{ + resp, err := callMethod(ctx, w, "IsAuthorized", func(inst api.Scrobbler) (*api.ScrobblerIsAuthorizedResponse, error) { + return inst.IsAuthorized(ctx, &api.ScrobblerIsAuthorizedRequest{ UserId: userId, Username: username, }) - if err != nil { - return false, err - } - if resp.Error != "" { - return false, nil - } - return resp.Authorized, nil }) - return err == nil && result + if err != nil { + log.Warn("Error calling IsAuthorized", "userId", userId, "pluginID", w.id, err) + } + return err == nil && resp.Authorized } func (w *wasmScrobblerPlugin) NowPlaying(ctx context.Context, userId string, track *model.MediaFile, position int) error { @@ -70,25 +65,7 @@ func (w *wasmScrobblerPlugin) NowPlaying(ctx context.Context, userId string, tra } } - artists := make([]*api.Artist, 0, len(track.Participants[model.RoleArtist])) - for _, a := range track.Participants[model.RoleArtist] { - artists = append(artists, &api.Artist{Name: a.Name, Mbid: a.MbzArtistID}) - } - albumArtists := make([]*api.Artist, 0, len(track.Participants[model.RoleAlbumArtist])) - for _, a := range track.Participants[model.RoleAlbumArtist] { - albumArtists = append(albumArtists, &api.Artist{Name: a.Name, Mbid: a.MbzArtistID}) - } - trackInfo := &api.TrackInfo{ - Id: track.ID, - Mbid: track.MbzRecordingID, - Name: track.Title, - Album: track.Album, - AlbumMbid: track.MbzAlbumID, - Artists: artists, - AlbumArtists: albumArtists, - Length: int32(track.Duration), - Position: int32(position), - } + trackInfo := w.toTrackInfo(track, position) _, err := callMethod(ctx, w, "NowPlaying", func(inst api.Scrobbler) (struct{}, error) { resp, err := inst.NowPlaying(ctx, &api.ScrobblerNowPlayingRequest{ UserId: userId, @@ -115,26 +92,7 @@ func (w *wasmScrobblerPlugin) Scrobble(ctx context.Context, userId string, s scr username = u.UserName } } - - track := &s.MediaFile - artists := make([]*api.Artist, 0, len(track.Participants[model.RoleArtist])) - for _, a := range track.Participants[model.RoleArtist] { - artists = append(artists, &api.Artist{Name: a.Name, Mbid: a.MbzArtistID}) - } - albumArtists := make([]*api.Artist, 0, len(track.Participants[model.RoleAlbumArtist])) - for _, a := range track.Participants[model.RoleAlbumArtist] { - albumArtists = append(albumArtists, &api.Artist{Name: a.Name, Mbid: a.MbzArtistID}) - } - trackInfo := &api.TrackInfo{ - Id: track.ID, - Mbid: track.MbzRecordingID, - Name: track.Title, - Album: track.Album, - AlbumMbid: track.MbzAlbumID, - Artists: artists, - AlbumArtists: albumArtists, - Length: int32(track.Duration), - } + trackInfo := w.toTrackInfo(&s.MediaFile, 0) _, err := callMethod(ctx, w, "Scrobble", func(inst api.Scrobbler) (struct{}, error) { resp, err := inst.Scrobble(ctx, &api.ScrobblerScrobbleRequest{ UserId: userId, @@ -152,3 +110,27 @@ func (w *wasmScrobblerPlugin) Scrobble(ctx context.Context, userId string, s scr }) return err } + +func (w *wasmScrobblerPlugin) toTrackInfo(track *model.MediaFile, position int) *api.TrackInfo { + artists := make([]*api.Artist, 0, len(track.Participants[model.RoleArtist])) + + for _, a := range track.Participants[model.RoleArtist] { + artists = append(artists, &api.Artist{Name: a.Name, Mbid: a.MbzArtistID}) + } + albumArtists := make([]*api.Artist, 0, len(track.Participants[model.RoleAlbumArtist])) + for _, a := range track.Participants[model.RoleAlbumArtist] { + albumArtists = append(albumArtists, &api.Artist{Name: a.Name, Mbid: a.MbzArtistID}) + } + trackInfo := &api.TrackInfo{ + Id: track.ID, + Mbid: track.MbzRecordingID, + Name: track.Title, + Album: track.Album, + AlbumMbid: track.MbzAlbumID, + Artists: artists, + AlbumArtists: albumArtists, + Length: int32(track.Duration), + Position: int32(position), + } + return trackInfo +} diff --git a/plugins/adapter_websocket_callback.go b/plugins/adapter_websocket_callback.go index c45ee342e..83b8dd567 100644 --- a/plugins/adapter_websocket_callback.go +++ b/plugins/adapter_websocket_callback.go @@ -9,14 +9,14 @@ import ( ) // newWasmWebSocketCallback creates a new adapter for a WebSocketCallback plugin -func newWasmWebSocketCallback(wasmPath, pluginID string, m *Manager, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin { +func newWasmWebSocketCallback(wasmPath, pluginID string, m *managerImpl, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin { loader, err := api.NewWebSocketCallbackPlugin(context.Background(), api.WazeroRuntime(runtime), api.WazeroModuleConfig(mc)) if err != nil { log.Error("Error creating WebSocket callback plugin", "plugin", pluginID, "path", wasmPath, err) return nil } return &wasmWebSocketCallback{ - wasmBasePlugin: newWasmBasePlugin[api.WebSocketCallback, *api.WebSocketCallbackPlugin]( + baseCapability: newBaseCapability[api.WebSocketCallback, *api.WebSocketCallbackPlugin]( wasmPath, pluginID, CapabilityWebSocketCallback, @@ -31,5 +31,5 @@ func newWasmWebSocketCallback(wasmPath, pluginID string, m *Manager, runtime api // wasmWebSocketCallback adapts a WebSocketCallback plugin type wasmWebSocketCallback struct { - *wasmBasePlugin[api.WebSocketCallback, *api.WebSocketCallbackPlugin] + *baseCapability[api.WebSocketCallback, *api.WebSocketCallbackPlugin] } diff --git a/plugins/api/errors.go b/plugins/api/errors.go index e6d952b4f..796774b15 100644 --- a/plugins/api/errors.go +++ b/plugins/api/errors.go @@ -3,6 +3,10 @@ package api import "errors" var ( - ErrNotFound = errors.New("plugin:not_found") + // ErrNotImplemented indicates that the plugin does not implement the requested method. + // No logic should be executed by the plugin. ErrNotImplemented = errors.New("plugin:not_implemented") + + // ErrNotFound indicates that the requested resource was not found by the plugin. + ErrNotFound = errors.New("plugin:not_found") ) diff --git a/plugins/base_capability.go b/plugins/base_capability.go new file mode 100644 index 000000000..6572a25ec --- /dev/null +++ b/plugins/base_capability.go @@ -0,0 +1,159 @@ +package plugins + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/navidrome/navidrome/core/metrics" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model/id" + "github.com/navidrome/navidrome/plugins/api" +) + +// newBaseCapability creates a new instance of baseCapability with the required parameters. +func newBaseCapability[S any, P any](wasmPath, id, capability string, m metrics.Metrics, loader P, loadFunc loaderFunc[S, P]) *baseCapability[S, P] { + return &baseCapability[S, P]{ + wasmPath: wasmPath, + id: id, + capability: capability, + loader: loader, + loadFunc: loadFunc, + metrics: m, + } +} + +// LoaderFunc is a generic function type that loads a plugin instance. +type loaderFunc[S any, P any] func(ctx context.Context, loader P, path string) (S, error) + +// baseCapability is a generic base implementation for WASM plugins. +// S is the capability interface type and P is the plugin loader type. +type baseCapability[S any, P any] struct { + wasmPath string + id string + capability string + loader P + loadFunc loaderFunc[S, P] + metrics metrics.Metrics +} + +func (w *baseCapability[S, P]) PluginID() string { + return w.id +} + +func (w *baseCapability[S, P]) serviceName() string { + return w.id + "_" + w.capability +} + +func (w *baseCapability[S, P]) getMetrics() metrics.Metrics { + return w.metrics +} + +// getInstance loads a new plugin instance and returns a cleanup function. +func (w *baseCapability[S, P]) getInstance(ctx context.Context, methodName string) (S, func(), error) { + start := time.Now() + // Add context metadata for tracing + ctx = log.NewContext(ctx, "capability", w.serviceName(), "method", methodName) + + inst, err := w.loadFunc(ctx, w.loader, w.wasmPath) + if err != nil { + var zero S + return zero, func() {}, fmt.Errorf("baseCapability: failed to load instance for %s: %w", w.serviceName(), err) + } + // Add context metadata for tracing + ctx = log.NewContext(ctx, "instanceID", getInstanceID(inst)) + log.Trace(ctx, "baseCapability: loaded instance", "elapsed", time.Since(start)) + return inst, func() { + log.Trace(ctx, "baseCapability: finished using instance", "elapsed", time.Since(start)) + if closer, ok := any(inst).(interface{ Close(context.Context) error }); ok { + _ = closer.Close(ctx) + } + }, nil +} + +type wasmPlugin[S any] interface { + PluginID() string + getInstance(ctx context.Context, methodName string) (S, func(), error) + getMetrics() metrics.Metrics +} + +func callMethod[S any, R any](ctx context.Context, wp WasmPlugin, methodName string, fn func(inst S) (R, error)) (R, error) { + // Add a unique call ID to the context for tracing + ctx = log.NewContext(ctx, "callID", id.NewRandom()) + var r R + + p, ok := wp.(wasmPlugin[S]) + if !ok { + log.Error(ctx, "callMethod: not a wasm plugin", "method", methodName, "pluginID", wp.PluginID()) + return r, fmt.Errorf("wasm plugin: not a wasm plugin: %s", wp.PluginID()) + } + + inst, done, err := p.getInstance(ctx, methodName) + if err != nil { + return r, err + } + start := time.Now() + defer done() + r, err = checkErr(fn(inst)) + elapsed := time.Since(start) + + if !errors.Is(err, api.ErrNotImplemented) { + id := p.PluginID() + isOk := err == nil + metrics := p.getMetrics() + if metrics != nil { + metrics.RecordPluginRequest(ctx, id, methodName, isOk, elapsed.Milliseconds()) + log.Trace(ctx, "callMethod: sending metrics", "plugin", id, "method", methodName, "ok", isOk, "elapsed", elapsed) + } + } + + return r, err +} + +// errorResponse is an interface that defines a method to retrieve an error message. +// It is automatically implemented (generated) by all plugin responses that have an Error field +type errorResponse interface { + GetError() string +} + +// checkErr returns an updated error if the response implements errorResponse and contains an error message. +// If the response is nil, it returns the original error. Otherwise, it wraps or creates an error as needed. +// It also maps error strings to their corresponding api.Err* constants. +func checkErr[T any](resp T, err error) (T, error) { + if any(resp) == nil { + return resp, mapAPIError(err) + } + respErr, ok := any(resp).(errorResponse) + if ok && respErr.GetError() != "" { + respErrMsg := respErr.GetError() + respErrErr := errors.New(respErrMsg) + mappedErr := mapAPIError(respErrErr) + // Check if the error was mapped to an API error (different from the temp error) + if errors.Is(mappedErr, api.ErrNotImplemented) || errors.Is(mappedErr, api.ErrNotFound) { + // Return the mapped API error instead of wrapping + return resp, mappedErr + } + // For non-API errors, use wrap the original error if it is not nil + return resp, errors.Join(respErrErr, err) + } + return resp, mapAPIError(err) +} + +// mapAPIError maps error strings to their corresponding api.Err* constants. +// This is needed as errors from plugins may not be of type api.Error, due to serialization/deserialization. +func mapAPIError(err error) error { + if err == nil { + return nil + } + + errStr := err.Error() + switch errStr { + case api.ErrNotImplemented.Error(): + return api.ErrNotImplemented + case api.ErrNotFound.Error(): + return api.ErrNotFound + default: + return err + } +} diff --git a/plugins/base_capability_test.go b/plugins/base_capability_test.go new file mode 100644 index 000000000..3bece8dcd --- /dev/null +++ b/plugins/base_capability_test.go @@ -0,0 +1,285 @@ +package plugins + +import ( + "context" + "errors" + + "github.com/navidrome/navidrome/plugins/api" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +type nilInstance struct{} + +var _ = Describe("baseCapability", func() { + var ctx = context.Background() + + It("should load instance using loadFunc", func() { + called := false + plugin := &baseCapability[*nilInstance, any]{ + wasmPath: "", + id: "test", + capability: "test", + loadFunc: func(ctx context.Context, _ any, path string) (*nilInstance, error) { + called = true + return &nilInstance{}, nil + }, + } + inst, done, err := plugin.getInstance(ctx, "test") + defer done() + Expect(err).To(BeNil()) + Expect(inst).ToNot(BeNil()) + Expect(called).To(BeTrue()) + }) +}) + +var _ = Describe("checkErr", func() { + Context("when resp is nil", func() { + It("should return nil error when both resp and err are nil", func() { + var resp *testErrorResponse + + result, err := checkErr(resp, nil) + + Expect(result).To(BeNil()) + Expect(err).To(BeNil()) + }) + + It("should return original error unchanged for non-API errors", func() { + var resp *testErrorResponse + originalErr := errors.New("original error") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(BeNil()) + Expect(err).To(Equal(originalErr)) + }) + + It("should return mapped API error for ErrNotImplemented", func() { + var resp *testErrorResponse + err := errors.New("plugin:not_implemented") + + result, mappedErr := checkErr(resp, err) + + Expect(result).To(BeNil()) + Expect(mappedErr).To(Equal(api.ErrNotImplemented)) + }) + + It("should return mapped API error for ErrNotFound", func() { + var resp *testErrorResponse + err := errors.New("plugin:not_found") + + result, mappedErr := checkErr(resp, err) + + Expect(result).To(BeNil()) + Expect(mappedErr).To(Equal(api.ErrNotFound)) + }) + }) + + Context("when resp is a typed nil that implements errorResponse", func() { + It("should not panic and return original error", func() { + var resp *testErrorResponse // typed nil + originalErr := errors.New("original error") + + // This should not panic + result, err := checkErr(resp, originalErr) + + Expect(result).To(BeNil()) + Expect(err).To(Equal(originalErr)) + }) + + It("should handle typed nil with nil error gracefully", func() { + var resp *testErrorResponse // typed nil + + // This should not panic + result, err := checkErr(resp, nil) + + Expect(result).To(BeNil()) + Expect(err).To(BeNil()) + }) + }) + + Context("when resp implements errorResponse with non-empty error", func() { + It("should create new error when original error is nil", func() { + resp := &testErrorResponse{errorMsg: "plugin error"} + + result, err := checkErr(resp, nil) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError("plugin error")) + }) + + It("should wrap original error when both exist", func() { + resp := &testErrorResponse{errorMsg: "plugin error"} + originalErr := errors.New("original error") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(HaveOccurred()) + // Check that both error messages are present in the joined error + errStr := err.Error() + Expect(errStr).To(ContainSubstring("plugin error")) + Expect(errStr).To(ContainSubstring("original error")) + }) + + It("should return mapped API error for ErrNotImplemented when no original error", func() { + resp := &testErrorResponse{errorMsg: "plugin:not_implemented"} + + result, err := checkErr(resp, nil) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError(api.ErrNotImplemented)) + }) + + It("should return mapped API error for ErrNotFound when no original error", func() { + resp := &testErrorResponse{errorMsg: "plugin:not_found"} + + result, err := checkErr(resp, nil) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError(api.ErrNotFound)) + }) + + It("should return mapped API error for ErrNotImplemented even with original error", func() { + resp := &testErrorResponse{errorMsg: "plugin:not_implemented"} + originalErr := errors.New("original error") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError(api.ErrNotImplemented)) + }) + + It("should return mapped API error for ErrNotFound even with original error", func() { + resp := &testErrorResponse{errorMsg: "plugin:not_found"} + originalErr := errors.New("original error") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError(api.ErrNotFound)) + }) + }) + + Context("when resp implements errorResponse with empty error", func() { + It("should return original error unchanged", func() { + resp := &testErrorResponse{errorMsg: ""} + originalErr := errors.New("original error") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError(originalErr)) + }) + + It("should return nil error when both are empty/nil", func() { + resp := &testErrorResponse{errorMsg: ""} + + result, err := checkErr(resp, nil) + + Expect(result).To(Equal(resp)) + Expect(err).To(BeNil()) + }) + + It("should map original API error when response error is empty", func() { + resp := &testErrorResponse{errorMsg: ""} + originalErr := errors.New("plugin:not_implemented") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError(api.ErrNotImplemented)) + }) + }) + + Context("when resp does not implement errorResponse", func() { + It("should return original error unchanged", func() { + resp := &testNonErrorResponse{data: "some data"} + originalErr := errors.New("original error") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(Equal(originalErr)) + }) + + It("should return nil error when original error is nil", func() { + resp := &testNonErrorResponse{data: "some data"} + + result, err := checkErr(resp, nil) + + Expect(result).To(Equal(resp)) + Expect(err).To(BeNil()) + }) + + It("should map original API error when response doesn't implement errorResponse", func() { + resp := &testNonErrorResponse{data: "some data"} + originalErr := errors.New("plugin:not_found") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError(api.ErrNotFound)) + }) + }) + + Context("when resp is a value type (not pointer)", func() { + It("should handle value types that implement errorResponse", func() { + resp := testValueErrorResponse{errorMsg: "value error"} + originalErr := errors.New("original error") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(HaveOccurred()) + // Check that both error messages are present in the joined error + errStr := err.Error() + Expect(errStr).To(ContainSubstring("value error")) + Expect(errStr).To(ContainSubstring("original error")) + }) + + It("should handle value types with empty error", func() { + resp := testValueErrorResponse{errorMsg: ""} + originalErr := errors.New("original error") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError(originalErr)) + }) + + It("should handle value types with API error", func() { + resp := testValueErrorResponse{errorMsg: "plugin:not_implemented"} + originalErr := errors.New("original error") + + result, err := checkErr(resp, originalErr) + + Expect(result).To(Equal(resp)) + Expect(err).To(MatchError(api.ErrNotImplemented)) + }) + }) +}) + +// Test helper types +type testErrorResponse struct { + errorMsg string +} + +func (t *testErrorResponse) GetError() string { + if t == nil { + return "" // This is what would typically happen with a typed nil + } + return t.errorMsg +} + +type testNonErrorResponse struct { + data string +} + +type testValueErrorResponse struct { + errorMsg string +} + +func (t testValueErrorResponse) GetError() string { + return t.errorMsg +} diff --git a/plugins/examples/README.md b/plugins/examples/README.md index 6527026fd..61d6b2ef9 100644 --- a/plugins/examples/README.md +++ b/plugins/examples/README.md @@ -4,11 +4,11 @@ This directory contains example plugins for Navidrome, intended for demonstratio ## Contents -- `wikimedia/`: Example plugin that retrieves artist information from Wikidata. -- `coverartarchive/`: Example plugin that retrieves album cover images from the Cover Art Archive. -- `crypto-ticker/`: Example plugin using websockets to log real-time cryptocurrency prices. -- `discord-rich-presence/`: Example plugin that integrates with Discord Rich Presence to display currently playing tracks on Discord profiles. -- `subsonicapi-demo/`: Example plugin that demonstrates how to interact with the Navidrome's Subsonic API from a plugin. +- `wikimedia/`: Retrieves artist information from Wikidata. +- `coverartarchive/`: Fetches album cover images from the Cover Art Archive. +- `crypto-ticker/`: Uses websockets to log real-time cryptocurrency prices. +- `discord-rich-presence/`: Integrates with Discord Rich Presence to display currently playing tracks on Discord profiles. +- `subsonicapi-demo/`: Demonstrates interaction with Navidrome's Subsonic API from a plugin. ## Building diff --git a/plugins/examples/coverartarchive/plugin.go b/plugins/examples/coverartarchive/plugin.go index f91546de3..ee612c31c 100644 --- a/plugins/examples/coverartarchive/plugin.go +++ b/plugins/examples/coverartarchive/plugin.go @@ -143,5 +143,9 @@ func (CoverArtArchiveAgent) GetArtistTopSongs(ctx context.Context, req *api.Arti func main() {} func init() { + // Configure logging: No timestamps, no source file/line + log.SetFlags(0) + log.SetPrefix("[CAA] ") + api.RegisterMetadataAgent(CoverArtArchiveAgent{}) } diff --git a/plugins/examples/crypto-ticker/README.md b/plugins/examples/crypto-ticker/README.md index c550ebfe9..ca6d2c44a 100644 --- a/plugins/examples/crypto-ticker/README.md +++ b/plugins/examples/crypto-ticker/README.md @@ -15,7 +15,7 @@ This is a WebSocket-based WASM plugin for Navidrome that displays real-time cryp In your `navidrome.toml` file, add: ```toml -[PluginSettings.crypto-ticker] +[PluginConfig.crypto-ticker] tickers = "BTC,ETH,SOL,MATIC" ``` diff --git a/plugins/examples/crypto-ticker/plugin.go b/plugins/examples/crypto-ticker/plugin.go index e7c646c21..3fced6d5c 100644 --- a/plugins/examples/crypto-ticker/plugin.go +++ b/plugins/examples/crypto-ticker/plugin.go @@ -294,6 +294,10 @@ func calculatePercentChange(open, current string) string { func main() {} func init() { + // Configure logging: No timestamps, no source file/line, prepend [Crypto] + log.SetFlags(0) + log.SetPrefix("[Crypto] ") + api.RegisterWebSocketCallback(CryptoTickerPlugin{}) api.RegisterLifecycleManagement(CryptoTickerPlugin{}) api.RegisterSchedulerCallback(CryptoTickerPlugin{}) diff --git a/plugins/examples/discord-rich-presence/rpc.go b/plugins/examples/discord-rich-presence/rpc.go index 4b383c53a..4fab42f41 100644 --- a/plugins/examples/discord-rich-presence/rpc.go +++ b/plugins/examples/discord-rich-presence/rpc.go @@ -248,9 +248,37 @@ func (r *discordRPC) sendHeartbeat(ctx context.Context, username string) error { return r.sendMessage(ctx, username, heartbeatOpCode, resp.Value) } +func (r *discordRPC) cleanupFailedConnection(ctx context.Context, username string) { + log.Printf("Cleaning up failed connection for user %s", username) + + // Cancel the heartbeat schedule + if resp, _ := r.sched.CancelSchedule(ctx, &scheduler.CancelRequest{ScheduleId: username}); resp.Error != "" { + log.Printf("Failed to cancel heartbeat schedule for user %s: %s", username, resp.Error) + } + + // Close the WebSocket connection + if resp, _ := r.ws.Close(ctx, &websocket.CloseRequest{ + ConnectionId: username, + Code: 1000, + Reason: "Connection lost", + }); resp.Error != "" { + log.Printf("Failed to close WebSocket connection for user %s: %s", username, resp.Error) + } + + // Clean up cache entries (just the sequence number, no failure tracking needed) + _, _ = r.mem.Remove(ctx, &cache.RemoveRequest{Key: fmt.Sprintf("discord.seq.%s", username)}) + + log.Printf("Cleaned up connection for user %s", username) +} + func (r *discordRPC) isConnected(ctx context.Context, username string) bool { + // Try to send a heartbeat to test the connection err := r.sendHeartbeat(ctx, username) - return err == nil + if err != nil { + log.Printf("Heartbeat test failed for user %s: %v", username, err) + return false + } + return true } func (r *discordRPC) connect(ctx context.Context, username string, token string) error { @@ -361,5 +389,14 @@ func (r *discordRPC) OnClose(_ context.Context, req *api.OnCloseRequest) (*api.O } func (r *discordRPC) OnSchedulerCallback(ctx context.Context, req *api.SchedulerCallbackRequest) (*api.SchedulerCallbackResponse, error) { - return nil, r.sendHeartbeat(ctx, req.ScheduleId) + err := r.sendHeartbeat(ctx, req.ScheduleId) + if err != nil { + // On first heartbeat failure, immediately clean up the connection + // The next NowPlaying call will reconnect if needed + log.Printf("Heartbeat failed for user %s, cleaning up connection: %v", req.ScheduleId, err) + r.cleanupFailedConnection(ctx, req.ScheduleId) + return nil, fmt.Errorf("heartbeat failed, connection cleaned up: %w", err) + } + + return nil, nil } diff --git a/plugins/examples/subsonicapi-demo/plugin.go b/plugins/examples/subsonicapi-demo/plugin.go index c3adc6579..4ca087ac7 100644 --- a/plugins/examples/subsonicapi-demo/plugin.go +++ b/plugins/examples/subsonicapi-demo/plugin.go @@ -60,5 +60,9 @@ func (SubsonicAPIDemoPlugin) OnInit(ctx context.Context, req *api.InitRequest) ( func main() {} func init() { + // Configure logging: No timestamps, no source file/line + log.SetFlags(0) + log.SetPrefix("[Subsonic Plugin] ") + api.RegisterLifecycleManagement(&SubsonicAPIDemoPlugin{}) } diff --git a/plugins/examples/wikimedia/plugin.go b/plugins/examples/wikimedia/plugin.go index b64e8cd86..6b60e69da 100644 --- a/plugins/examples/wikimedia/plugin.go +++ b/plugins/examples/wikimedia/plugin.go @@ -383,5 +383,9 @@ func (WikimediaAgent) GetAlbumImages(context.Context, *api.AlbumImagesRequest) ( func main() {} func init() { + // Configure logging: No timestamps, no source file/line + log.SetFlags(0) + log.SetPrefix("[Wikimedia] ") + api.RegisterMetadataAgent(WikimediaAgent{}) } diff --git a/plugins/host_scheduler.go b/plugins/host_scheduler.go index 6cea93280..e3585990a 100644 --- a/plugins/host_scheduler.go +++ b/plugins/host_scheduler.go @@ -8,7 +8,6 @@ import ( gonanoid "github.com/matoous/go-nanoid/v2" "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/plugins/api" "github.com/navidrome/navidrome/plugins/host/scheduler" navidsched "github.com/navidrome/navidrome/scheduler" ) @@ -49,13 +48,13 @@ func (s SchedulerHostFunctions) CancelSchedule(ctx context.Context, req *schedul type schedulerService struct { // Map of schedule IDs to their callback info schedules map[string]*ScheduledCallback - manager *Manager + manager *managerImpl navidSched navidsched.Scheduler // Navidrome scheduler for recurring jobs mu sync.Mutex } // newSchedulerService creates a new schedulerService instance -func newSchedulerService(manager *Manager) *schedulerService { +func newSchedulerService(manager *managerImpl) *schedulerService { return &schedulerService{ schedules: make(map[string]*ScheduledCallback), manager: manager, @@ -295,21 +294,10 @@ func (s *schedulerService) executeCallback(ctx context.Context, internalSchedule return } - callbackType := "one-time" - if isRecurring { - callbackType = "recurring" - } - - log.Debug("Executing schedule callback", "plugin", callback.PluginID, "scheduleID", callback.ID, "type", callbackType) + ctx = log.NewContext(ctx, "plugin", callback.PluginID, "scheduleID", callback.ID, "type", callback.Type) + log.Debug("Executing schedule callback") start := time.Now() - // Create a SchedulerCallbackRequest - req := &api.SchedulerCallbackRequest{ - ScheduleId: callback.ID, - Payload: callback.Payload, - IsRecurring: isRecurring, - } - // Get the plugin p := s.manager.LoadPlugin(callback.PluginID, CapabilitySchedulerCallback) if p == nil { @@ -317,31 +305,19 @@ func (s *schedulerService) executeCallback(ctx context.Context, internalSchedule return } - // Get instance - inst, closeFn, err := p.Instantiate(ctx) - if err != nil { - log.Error("Error getting plugin instance for callback", "plugin", callback.PluginID, err) - return - } - defer closeFn() - // Type-check the plugin - plugin, ok := inst.(api.SchedulerCallback) + plugin, ok := p.(*wasmSchedulerCallback) if !ok { log.Error("Plugin does not implement SchedulerCallback", "plugin", callback.PluginID) return } // Call the plugin's OnSchedulerCallback method - log.Trace(ctx, "Executing schedule callback", "plugin", callback.PluginID, "scheduleID", callback.ID, "type", callbackType) - resp, err := plugin.OnSchedulerCallback(ctx, req) + log.Trace(ctx, "Executing schedule callback") + err := plugin.OnSchedulerCallback(ctx, callback.ID, callback.Payload, isRecurring) if err != nil { - log.Error("Error executing schedule callback", "plugin", callback.PluginID, "elapsed", time.Since(start), err) + log.Error("Error executing schedule callback", "elapsed", time.Since(start), err) return } - log.Debug("Schedule callback executed", "plugin", callback.PluginID, "elapsed", time.Since(start)) - - if resp.Error != "" { - log.Error("Plugin reported error in schedule callback", "plugin", callback.PluginID, resp.Error) - } + log.Debug("Schedule callback executed", "elapsed", time.Since(start)) } diff --git a/plugins/host_scheduler_test.go b/plugins/host_scheduler_test.go index f544d716e..a905313b7 100644 --- a/plugins/host_scheduler_test.go +++ b/plugins/host_scheduler_test.go @@ -3,6 +3,7 @@ package plugins import ( "context" + "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/plugins/host/scheduler" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -11,12 +12,12 @@ import ( var _ = Describe("SchedulerService", func() { var ( ss *schedulerService - manager *Manager + manager *managerImpl pluginName = "test_plugin" ) BeforeEach(func() { - manager = createManager(nil, nil) + manager = createManager(nil, metrics.NewNoopInstance()) ss = manager.schedulerService }) diff --git a/plugins/host_websocket.go b/plugins/host_websocket.go index 131596b94..e90d1363d 100644 --- a/plugins/host_websocket.go +++ b/plugins/host_websocket.go @@ -50,12 +50,12 @@ func (s WebSocketHostFunctions) Close(ctx context.Context, req *websocket.CloseR // websocketService implements the WebSocket service functionality type websocketService struct { connections map[string]*WebSocketConnection - manager *Manager + manager *managerImpl mu sync.RWMutex } // newWebsocketService creates a new websocketService instance -func newWebsocketService(manager *Manager) *websocketService { +func newWebsocketService(manager *managerImpl) *websocketService { return &websocketService{ connections: make(map[string]*WebSocketConnection), manager: manager, @@ -314,7 +314,7 @@ func (s *websocketService) handleMessages(internalID string, conn *WebSocketConn // executeCallback is a common function that handles the plugin loading and execution // for all types of callbacks -func (s *websocketService) executeCallback(ctx context.Context, pluginID string, fn func(context.Context, api.WebSocketCallback) error) { +func (s *websocketService) executeCallback(ctx context.Context, pluginID, methodName string, fn func(context.Context, api.WebSocketCallback) error) { log.Debug(ctx, "WebSocket received") start := time.Now() @@ -326,30 +326,16 @@ func (s *websocketService) executeCallback(ctx context.Context, pluginID string, return } - // Get instance - inst, closeFn, err := p.Instantiate(ctx) - if err != nil { - log.Error(ctx, "Error getting plugin instance for WebSocket callback", err) - return - } - defer closeFn() - - // Type-check the plugin - plugin, ok := inst.(api.WebSocketCallback) - if !ok { - log.Error(ctx, "Plugin does not implement WebSocketCallback") - return - } - - // Call the appropriate callback function - log.Trace(ctx, "Executing WebSocket callback") - - if err = fn(ctx, plugin); err != nil { - log.Error(ctx, "Error executing WebSocket callback", "elapsed", time.Since(start), err) - return - } - - log.Debug(ctx, "WebSocket callback executed", "elapsed", time.Since(start)) + _, _ = callMethod(ctx, p, methodName, func(inst api.WebSocketCallback) (struct{}, error) { + // Call the appropriate callback function + log.Trace(ctx, "Executing WebSocket callback") + if err := fn(ctx, inst); err != nil { + log.Error(ctx, "Error executing WebSocket callback", "elapsed", time.Since(start), err) + return struct{}{}, fmt.Errorf("error executing WebSocket callback: %w", err) + } + log.Debug(ctx, "WebSocket callback executed", "elapsed", time.Since(start)) + return struct{}{}, nil + }) } // notifyTextCallback notifies the plugin of a text message @@ -361,8 +347,8 @@ func (s *websocketService) notifyTextCallback(ctx context.Context, connectionID ctx = log.NewContext(ctx, "callback", "OnTextMessage", "size", len(message)) - s.executeCallback(ctx, conn.PluginName, func(ctx context.Context, plugin api.WebSocketCallback) error { - _, err := plugin.OnTextMessage(ctx, req) + s.executeCallback(ctx, conn.PluginName, "OnTextMessage", func(ctx context.Context, plugin api.WebSocketCallback) error { + _, err := checkErr(plugin.OnTextMessage(ctx, req)) return err }) } @@ -376,8 +362,8 @@ func (s *websocketService) notifyBinaryCallback(ctx context.Context, connectionI ctx = log.NewContext(ctx, "callback", "OnBinaryMessage", "size", len(data)) - s.executeCallback(ctx, conn.PluginName, func(ctx context.Context, plugin api.WebSocketCallback) error { - _, err := plugin.OnBinaryMessage(ctx, req) + s.executeCallback(ctx, conn.PluginName, "OnBinaryMessage", func(ctx context.Context, plugin api.WebSocketCallback) error { + _, err := checkErr(plugin.OnBinaryMessage(ctx, req)) return err }) } @@ -391,8 +377,8 @@ func (s *websocketService) notifyErrorCallback(ctx context.Context, connectionID ctx = log.NewContext(ctx, "callback", "OnError", "error", errorMsg) - s.executeCallback(ctx, conn.PluginName, func(ctx context.Context, plugin api.WebSocketCallback) error { - _, err := plugin.OnError(ctx, req) + s.executeCallback(ctx, conn.PluginName, "OnError", func(ctx context.Context, plugin api.WebSocketCallback) error { + _, err := checkErr(plugin.OnError(ctx, req)) return err }) } @@ -407,8 +393,8 @@ func (s *websocketService) notifyCloseCallback(ctx context.Context, connectionID ctx = log.NewContext(ctx, "callback", "OnClose", "code", code, "reason", reason) - s.executeCallback(ctx, conn.PluginName, func(ctx context.Context, plugin api.WebSocketCallback) error { - _, err := plugin.OnClose(ctx, req) + s.executeCallback(ctx, conn.PluginName, "OnClose", func(ctx context.Context, plugin api.WebSocketCallback) error { + _, err := checkErr(plugin.OnClose(ctx, req)) return err }) } diff --git a/plugins/host_websocket_test.go b/plugins/host_websocket_test.go index b6f4e2094..ecadc6463 100644 --- a/plugins/host_websocket_test.go +++ b/plugins/host_websocket_test.go @@ -6,9 +6,11 @@ import ( "net/http/httptest" "strings" "sync" + "testing" "time" gorillaws "github.com/gorilla/websocket" + "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/plugins/host/websocket" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -17,7 +19,7 @@ import ( var _ = Describe("WebSocket Host Service", func() { var ( wsService *websocketService - manager *Manager + manager *managerImpl ctx context.Context server *httptest.Server upgrader gorillaws.Upgrader @@ -84,7 +86,7 @@ var _ = Describe("WebSocket Host Service", func() { DeferCleanup(server.Close) // Create a new manager and websocket service - manager = createManager(nil, nil) + manager = createManager(nil, metrics.NewNoopInstance()) wsService = newWebsocketService(manager) }) @@ -188,6 +190,10 @@ var _ = Describe("WebSocket Host Service", func() { }) It("handles connection errors gracefully", func() { + if testing.Short() { + GinkgoT().Skip("skipping test in short mode.") + } + // Try to connect to an invalid URL req := &websocket.ConnectRequest{ Url: "ws://invalid-url-that-does-not-exist", diff --git a/plugins/manager.go b/plugins/manager.go index 89ff854ae..0800d2744 100644 --- a/plugins/manager.go +++ b/plugins/manager.go @@ -10,10 +10,10 @@ package plugins //go:generate protoc --go-plugin_out=. --go-plugin_opt=paths=source_relative host/subsonicapi/subsonicapi.proto import ( - "context" "fmt" "net/http" "os" + "slices" "sync" "sync/atomic" "time" @@ -40,7 +40,7 @@ const ( ) // pluginCreators maps capability types to their respective creator functions -type pluginConstructor func(wasmPath, pluginID string, m *Manager, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin +type pluginConstructor func(wasmPath, pluginID string, m *managerImpl, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin var pluginCreators = map[string]pluginConstructor{ CapabilityMetadataAgent: newWasmMediaAgent, @@ -53,8 +53,6 @@ var pluginCreators = map[string]pluginConstructor{ type WasmPlugin interface { // PluginID returns the unique identifier of the plugin (folder name) PluginID() string - // Instantiate creates a new instance of the plugin and returns it along with a cleanup function - Instantiate(ctx context.Context) (any, func(), error) } type plugin struct { @@ -86,8 +84,18 @@ func (p *plugin) waitForCompilation() error { type SubsonicRouter http.Handler -// Manager is a singleton that manages plugins -type Manager struct { +type Manager interface { + SetSubsonicRouter(router SubsonicRouter) + EnsureCompiled(name string) error + PluginNames(serviceName string) []string + LoadPlugin(name string, capability string) WasmPlugin + LoadMediaAgent(name string) (agents.Interface, bool) + LoadScrobbler(name string) (scrobbler.Scrobbler, bool) + ScanPlugins() +} + +// managerImpl is a singleton that manages plugins +type managerImpl struct { plugins map[string]*plugin // Map of plugin folder name to plugin info mu sync.RWMutex // Protects plugins map subsonicRouter atomic.Pointer[SubsonicRouter] // Subsonic API router @@ -99,18 +107,21 @@ type Manager struct { metrics metrics.Metrics } -// GetManager returns the singleton instance of Manager -func GetManager(ds model.DataStore, metrics metrics.Metrics) *Manager { - return singleton.GetInstance(func() *Manager { +// GetManager returns the singleton instance of managerImpl +func GetManager(ds model.DataStore, metrics metrics.Metrics) Manager { + if !conf.Server.Plugins.Enabled { + return &noopManager{} + } + return singleton.GetInstance(func() *managerImpl { return createManager(ds, metrics) }) } -// createManager creates a new Manager instance. Used in tests -func createManager(ds model.DataStore, metrics metrics.Metrics) *Manager { - m := &Manager{ +// createManager creates a new managerImpl instance. Used in tests +func createManager(ds model.DataStore, metrics metrics.Metrics) *managerImpl { + m := &managerImpl{ plugins: make(map[string]*plugin), - lifecycle: newPluginLifecycleManager(), + lifecycle: newPluginLifecycleManager(metrics), ds: ds, metrics: metrics, } @@ -122,14 +133,14 @@ func createManager(ds model.DataStore, metrics metrics.Metrics) *Manager { return m } -// SetSubsonicRouter sets the SubsonicRouter after Manager initialization -func (m *Manager) SetSubsonicRouter(router SubsonicRouter) { +// SetSubsonicRouter sets the SubsonicRouter after managerImpl initialization +func (m *managerImpl) SetSubsonicRouter(router SubsonicRouter) { m.subsonicRouter.Store(&router) } // registerPlugin adds a plugin to the registry with the given parameters // Used internally by ScanPlugins to register plugins -func (m *Manager) registerPlugin(pluginID, pluginDir, wasmPath string, manifest *schema.PluginManifest) *plugin { +func (m *managerImpl) registerPlugin(pluginID, pluginDir, wasmPath string, manifest *schema.PluginManifest) *plugin { // Create custom runtime function customRuntime := m.createRuntime(pluginID, manifest.Permissions) @@ -154,16 +165,8 @@ func (m *Manager) registerPlugin(pluginID, pluginDir, wasmPath string, manifest compilationReady: make(chan struct{}), } - // Start pre-compilation of WASM module in background - go func() { - precompilePlugin(p) - // Check if this plugin implements InitService and hasn't been initialized yet - m.initializePluginIfNeeded(p) - }() - - // Register the plugin + // Register the plugin first m.mu.Lock() - defer m.mu.Unlock() m.plugins[pluginID] = p // Register one plugin adapter for each capability @@ -184,30 +187,59 @@ func (m *Manager) registerPlugin(pluginID, pluginDir, wasmPath string, manifest } m.adapters[pluginID+"_"+capabilityStr] = adapter } + m.mu.Unlock() + + // Start pre-compilation of WASM module in background AFTER registration + go func() { + precompilePlugin(p) + // Check if this plugin implements InitService and hasn't been initialized yet + m.initializePluginIfNeeded(p) + }() log.Info("Discovered plugin", "folder", pluginID, "name", manifest.Name, "capabilities", manifest.Capabilities, "wasm", wasmPath, "dev_mode", isSymlink) return m.plugins[pluginID] } // initializePluginIfNeeded calls OnInit on plugins that implement LifecycleManagement -func (m *Manager) initializePluginIfNeeded(plugin *plugin) { +func (m *managerImpl) initializePluginIfNeeded(plugin *plugin) { // Skip if already initialized if m.lifecycle.isInitialized(plugin) { return } // Check if the plugin implements LifecycleManagement - for _, capability := range plugin.Manifest.Capabilities { - if capability == CapabilityLifecycleManagement { - m.lifecycle.callOnInit(plugin) - m.lifecycle.markInitialized(plugin) - break + if slices.Contains(plugin.Manifest.Capabilities, CapabilityLifecycleManagement) { + if err := m.lifecycle.callOnInit(plugin); err != nil { + m.unregisterPlugin(plugin.ID) } } } +// unregisterPlugin removes a plugin from the manager +func (m *managerImpl) unregisterPlugin(pluginID string) { + m.mu.Lock() + defer m.mu.Unlock() + + plugin, ok := m.plugins[pluginID] + if !ok { + return + } + + // Clear initialization state from lifecycle manager + m.lifecycle.clearInitialized(plugin) + + // Unregister plugin adapters + for _, capability := range plugin.Manifest.Capabilities { + delete(m.adapters, pluginID+"_"+string(capability)) + } + + // Unregister plugin + delete(m.plugins, pluginID) + log.Info("Unregistered plugin", "plugin", pluginID) +} + // ScanPlugins scans the plugins directory, discovers all valid plugins, and registers them for use. -func (m *Manager) ScanPlugins() { +func (m *managerImpl) ScanPlugins() { // Clear existing plugins m.mu.Lock() m.plugins = make(map[string]*plugin) @@ -259,7 +291,7 @@ func (m *Manager) ScanPlugins() { } // PluginNames returns the folder names of all plugins that implement the specified capability -func (m *Manager) PluginNames(capability string) []string { +func (m *managerImpl) PluginNames(capability string) []string { m.mu.RLock() defer m.mu.RUnlock() @@ -275,28 +307,26 @@ func (m *Manager) PluginNames(capability string) []string { return names } -func (m *Manager) getPlugin(name string, capability string) (*plugin, WasmPlugin) { +func (m *managerImpl) getPlugin(name string, capability string) (*plugin, WasmPlugin, error) { m.mu.RLock() defer m.mu.RUnlock() info, infoOk := m.plugins[name] adapter, adapterOk := m.adapters[name+"_"+capability] if !infoOk { - log.Warn("Plugin not found", "name", name) - return nil, nil + return nil, nil, fmt.Errorf("plugin not registered: %s", name) } if !adapterOk { - log.Warn("Plugin adapter not found", "name", name, "capability", capability) - return nil, nil + return nil, nil, fmt.Errorf("plugin adapter not registered: %s, capability: %s", name, capability) } - return info, adapter + return info, adapter, nil } // LoadPlugin instantiates and returns a plugin by folder name -func (m *Manager) LoadPlugin(name string, capability string) WasmPlugin { - info, adapter := m.getPlugin(name, capability) - if info == nil { - log.Warn("Plugin not found", "name", name, "capability", capability) +func (m *managerImpl) LoadPlugin(name string, capability string) WasmPlugin { + info, adapter, err := m.getPlugin(name, capability) + if err != nil { + log.Warn("Error loading plugin", err) return nil } @@ -318,7 +348,7 @@ func (m *Manager) LoadPlugin(name string, capability string) WasmPlugin { // EnsureCompiled waits for a plugin to finish compilation and returns any compilation error. // This is useful when you need to wait for compilation without loading a specific capability, // such as during plugin refresh operations or health checks. -func (m *Manager) EnsureCompiled(name string) error { +func (m *managerImpl) EnsureCompiled(name string) error { m.mu.RLock() plugin, ok := m.plugins[name] m.mu.RUnlock() @@ -330,25 +360,8 @@ func (m *Manager) EnsureCompiled(name string) error { return plugin.waitForCompilation() } -// LoadAllPlugins instantiates and returns all plugins that implement the specified capability -func (m *Manager) LoadAllPlugins(capability string) []WasmPlugin { - names := m.PluginNames(capability) - if len(names) == 0 { - return nil - } - - var plugins []WasmPlugin - for _, name := range names { - plugin := m.LoadPlugin(name, capability) - if plugin != nil { - plugins = append(plugins, plugin) - } - } - return plugins -} - // LoadMediaAgent instantiates and returns a media agent plugin by folder name -func (m *Manager) LoadMediaAgent(name string) (agents.Interface, bool) { +func (m *managerImpl) LoadMediaAgent(name string) (agents.Interface, bool) { plugin := m.LoadPlugin(name, CapabilityMetadataAgent) if plugin == nil { return nil, false @@ -357,17 +370,8 @@ func (m *Manager) LoadMediaAgent(name string) (agents.Interface, bool) { return agent, ok } -// LoadAllMediaAgents instantiates and returns all media agent plugins -func (m *Manager) LoadAllMediaAgents() []agents.Interface { - plugins := m.LoadAllPlugins(CapabilityMetadataAgent) - - return slice.Map(plugins, func(p WasmPlugin) agents.Interface { - return p.(agents.Interface) - }) -} - // LoadScrobbler instantiates and returns a scrobbler plugin by folder name -func (m *Manager) LoadScrobbler(name string) (scrobbler.Scrobbler, bool) { +func (m *managerImpl) LoadScrobbler(name string) (scrobbler.Scrobbler, bool) { plugin := m.LoadPlugin(name, CapabilityScrobbler) if plugin == nil { return nil, false @@ -376,11 +380,18 @@ func (m *Manager) LoadScrobbler(name string) (scrobbler.Scrobbler, bool) { return s, ok } -// LoadAllScrobblers instantiates and returns all scrobbler plugins -func (m *Manager) LoadAllScrobblers() []scrobbler.Scrobbler { - plugins := m.LoadAllPlugins(CapabilityScrobbler) +type noopManager struct{} - return slice.Map(plugins, func(p WasmPlugin) scrobbler.Scrobbler { - return p.(scrobbler.Scrobbler) - }) -} +func (n noopManager) SetSubsonicRouter(router SubsonicRouter) {} + +func (n noopManager) EnsureCompiled(name string) error { return nil } + +func (n noopManager) PluginNames(serviceName string) []string { return nil } + +func (n noopManager) LoadPlugin(name string, capability string) WasmPlugin { return nil } + +func (n noopManager) LoadMediaAgent(name string) (agents.Interface, bool) { return nil, false } + +func (n noopManager) LoadScrobbler(name string) (scrobbler.Scrobbler, bool) { return nil, false } + +func (n noopManager) ScanPlugins() {} diff --git a/plugins/manager_test.go b/plugins/manager_test.go index 55a3b8f72..9445979c2 100644 --- a/plugins/manager_test.go +++ b/plugins/manager_test.go @@ -7,12 +7,14 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/core/metrics" + "github.com/navidrome/navidrome/plugins/schema" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) var _ = Describe("Plugin Manager", func() { - var mgr *Manager + var mgr *managerImpl var ctx context.Context BeforeEach(func() { @@ -27,7 +29,7 @@ var _ = Describe("Plugin Manager", func() { conf.Server.Plugins.Folder = testDataDir ctx = GinkgoT().Context() - mgr = createManager(nil, nil) + mgr = createManager(nil, metrics.NewNoopInstance()) mgr.ScanPlugins() }) @@ -36,17 +38,21 @@ var _ = Describe("Plugin Manager", func() { mediaAgentNames := mgr.PluginNames("MetadataAgent") Expect(mediaAgentNames).To(HaveLen(4)) - Expect(mediaAgentNames).To(ContainElement("fake_artist_agent")) - Expect(mediaAgentNames).To(ContainElement("fake_album_agent")) - Expect(mediaAgentNames).To(ContainElement("multi_plugin")) - Expect(mediaAgentNames).To(ContainElement("unauthorized_plugin")) + Expect(mediaAgentNames).To(ContainElements( + "fake_artist_agent", + "fake_album_agent", + "multi_plugin", + "unauthorized_plugin", + )) scrobblerNames := mgr.PluginNames("Scrobbler") Expect(scrobblerNames).To(ContainElement("fake_scrobbler")) initServiceNames := mgr.PluginNames("LifecycleManagement") - Expect(initServiceNames).To(ContainElement("multi_plugin")) - Expect(initServiceNames).To(ContainElement("fake_init_service")) + Expect(initServiceNames).To(ContainElements("multi_plugin", "fake_init_service")) + + schedulerCallbackNames := mgr.PluginNames("SchedulerCallback") + Expect(schedulerCallbackNames).To(ContainElement("multi_plugin")) }) It("should load a MetadataAgent plugin and invoke artist-related methods", func() { @@ -65,18 +71,23 @@ var _ = Describe("Plugin Manager", func() { }) It("should load all MetadataAgent plugins", func() { - agents := mgr.LoadAllMediaAgents() - Expect(agents).To(HaveLen(4)) - var names []string - for _, a := range agents { - names = append(names, a.AgentName()) + mediaAgentNames := mgr.PluginNames("MetadataAgent") + Expect(mediaAgentNames).To(HaveLen(4)) + + var agentNames []string + for _, name := range mediaAgentNames { + agent, ok := mgr.LoadMediaAgent(name) + if ok { + agentNames = append(agentNames, agent.AgentName()) + } } - Expect(names).To(ContainElements("fake_artist_agent", "fake_album_agent", "multi_plugin", "unauthorized_plugin")) + + Expect(agentNames).To(ContainElements("fake_artist_agent", "fake_album_agent", "multi_plugin", "unauthorized_plugin")) }) Describe("ScanPlugins", func() { var tempPluginsDir string - var m *Manager + var m *managerImpl BeforeEach(func() { tempPluginsDir, _ = os.MkdirTemp("", "navidrome-plugins-test-*") @@ -85,7 +96,7 @@ var _ = Describe("Plugin Manager", func() { }) conf.Server.Plugins.Folder = tempPluginsDir - m = createManager(nil, nil) + m = createManager(nil, metrics.NewNoopInstance()) }) // Helper to create a complete valid plugin for manager testing @@ -193,21 +204,8 @@ var _ = Describe("Plugin Manager", func() { Describe("Invoke Methods", func() { It("should load all MetadataAgent plugins and invoke methods", func() { - mediaAgentNames := mgr.PluginNames("MetadataAgent") - Expect(mediaAgentNames).NotTo(BeEmpty()) - - plugins := mgr.LoadAllPlugins("MetadataAgent") - Expect(plugins).To(HaveLen(len(mediaAgentNames))) - - var fakeAlbumPlugin agents.Interface - for _, p := range plugins { - if agent, ok := p.(agents.Interface); ok { - if agent.AgentName() == "fake_album_agent" { - fakeAlbumPlugin = agent - break - } - } - } + fakeAlbumPlugin, isMediaAgent := mgr.LoadMediaAgent("fake_album_agent") + Expect(isMediaAgent).To(BeTrue()) Expect(fakeAlbumPlugin).NotTo(BeNil(), "fake_album_agent should be loaded") @@ -254,4 +252,95 @@ var _ = Describe("Plugin Manager", func() { } }) }) + + Describe("Plugin Initialization Lifecycle", func() { + BeforeEach(func() { + conf.Server.Plugins.Enabled = true + conf.Server.Plugins.Folder = testDataDir + }) + + Context("when OnInit is successful", func() { + It("should register and initialize the plugin", func() { + conf.Server.PluginConfig = nil + mgr = createManager(nil, metrics.NewNoopInstance()) // Create manager after setting config + mgr.ScanPlugins() + + plugin := mgr.plugins["fake_init_service"] + Expect(plugin).NotTo(BeNil()) + + Eventually(func() bool { + return mgr.lifecycle.isInitialized(plugin) + }).Should(BeTrue()) + + // Check that the plugin is still registered + names := mgr.PluginNames(CapabilityLifecycleManagement) + Expect(names).To(ContainElement("fake_init_service")) + }) + }) + + Context("when OnInit fails", func() { + It("should unregister the plugin if OnInit returns an error string", func() { + conf.Server.PluginConfig = map[string]map[string]string{ + "fake_init_service": { + "returnError": "response_error", + }, + } + mgr = createManager(nil, metrics.NewNoopInstance()) // Create manager after setting config + mgr.ScanPlugins() + + Eventually(func() []string { + return mgr.PluginNames(CapabilityLifecycleManagement) + }).ShouldNot(ContainElement("fake_init_service")) + }) + + It("should unregister the plugin if OnInit returns a Go error", func() { + conf.Server.PluginConfig = map[string]map[string]string{ + "fake_init_service": { + "returnError": "go_error", + }, + } + mgr = createManager(nil, metrics.NewNoopInstance()) // Create manager after setting config + mgr.ScanPlugins() + + Eventually(func() []string { + return mgr.PluginNames(CapabilityLifecycleManagement) + }).ShouldNot(ContainElement("fake_init_service")) + }) + }) + + It("should clear lifecycle state when unregistering a plugin", func() { + // Create a manager and register a plugin + mgr := createManager(nil, metrics.NewNoopInstance()) + + // Create a mock plugin with LifecycleManagement capability + plugin := &plugin{ + ID: "test-plugin", + Capabilities: []string{CapabilityLifecycleManagement}, + Manifest: &schema.PluginManifest{ + Version: "1.0.0", + }, + } + + // Register the plugin in the manager + mgr.mu.Lock() + mgr.plugins[plugin.ID] = plugin + mgr.mu.Unlock() + + // Mark the plugin as initialized in the lifecycle manager + mgr.lifecycle.markInitialized(plugin) + Expect(mgr.lifecycle.isInitialized(plugin)).To(BeTrue()) + + // Unregister the plugin + mgr.unregisterPlugin(plugin.ID) + + // Verify that the plugin is no longer in the manager + mgr.mu.RLock() + _, exists := mgr.plugins[plugin.ID] + mgr.mu.RUnlock() + Expect(exists).To(BeFalse()) + + // Verify that the lifecycle state has been cleared + Expect(mgr.lifecycle.isInitialized(plugin)).To(BeFalse()) + }) + }) }) diff --git a/plugins/manifest_permissions_test.go b/plugins/manifest_permissions_test.go index da221eb56..7a3df5f2d 100644 --- a/plugins/manifest_permissions_test.go +++ b/plugins/manifest_permissions_test.go @@ -8,6 +8,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/plugins/schema" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -47,7 +48,7 @@ func createTestPlugin(tempDir, name string, permissions schema.PluginManifestPer var _ = Describe("Plugin Permissions", func() { var ( - mgr *Manager + mgr *managerImpl tempDir string ctx context.Context ) @@ -55,7 +56,7 @@ var _ = Describe("Plugin Permissions", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) ctx = context.Background() - mgr = createManager(nil, nil) + mgr = createManager(nil, metrics.NewNoopInstance()) tempDir = GinkgoT().TempDir() }) diff --git a/plugins/plugin_lifecycle_manager.go b/plugins/plugin_lifecycle_manager.go index 7df0921d8..e00e7e5f3 100644 --- a/plugins/plugin_lifecycle_manager.go +++ b/plugins/plugin_lifecycle_manager.go @@ -8,6 +8,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/plugins/api" ) @@ -16,13 +17,15 @@ import ( type pluginLifecycleManager struct { plugins sync.Map // string -> bool config map[string]map[string]string + metrics metrics.Metrics } // newPluginLifecycleManager creates a new plugin lifecycle manager -func newPluginLifecycleManager() *pluginLifecycleManager { +func newPluginLifecycleManager(metrics metrics.Metrics) *pluginLifecycleManager { config := maps.Clone(conf.Server.PluginConfig) return &pluginLifecycleManager{ - config: config, + config: config, + metrics: metrics, } } @@ -39,8 +42,14 @@ func (m *pluginLifecycleManager) markInitialized(plugin *plugin) { m.plugins.Store(key, true) } +// clearInitialized removes the initialization state of a plugin +func (m *pluginLifecycleManager) clearInitialized(plugin *plugin) { + key := plugin.ID + consts.Zwsp + plugin.Manifest.Version + m.plugins.Delete(key) +} + // callOnInit calls the OnInit method on a plugin that implements LifecycleManagement -func (m *pluginLifecycleManager) callOnInit(plugin *plugin) { +func (m *pluginLifecycleManager) callOnInit(plugin *plugin) error { ctx := context.Background() log.Debug("Initializing plugin", "name", plugin.ID) start := time.Now() @@ -49,13 +58,13 @@ func (m *pluginLifecycleManager) callOnInit(plugin *plugin) { loader, err := api.NewLifecycleManagementPlugin(ctx, api.WazeroRuntime(plugin.Runtime), api.WazeroModuleConfig(plugin.ModConfig)) if loader == nil || err != nil { log.Error("Error creating LifecycleManagement plugin", "plugin", plugin.ID, err) - return + return err } initPlugin, err := loader.Load(ctx, plugin.WasmPath) if err != nil { log.Error("Error loading LifecycleManagement plugin", "plugin", plugin.ID, "path", plugin.WasmPath, err) - return + return err } defer initPlugin.Close(ctx) @@ -71,16 +80,16 @@ func (m *pluginLifecycleManager) callOnInit(plugin *plugin) { } // Call OnInit - resp, err := initPlugin.OnInit(ctx, req) + callStart := time.Now() + _, err = checkErr(initPlugin.OnInit(ctx, req)) + m.metrics.RecordPluginRequest(ctx, plugin.ID, "OnInit", err == nil, time.Since(callStart).Milliseconds()) if err != nil { log.Error("Error initializing plugin", "plugin", plugin.ID, "elapsed", time.Since(start), err) - return - } - - if resp.Error != "" { - log.Error("Plugin reported error during initialization", "plugin", plugin.ID, "error", resp.Error) - return + return err } + // Mark the plugin as initialized + m.markInitialized(plugin) log.Debug("Plugin initialized successfully", "plugin", plugin.ID, "elapsed", time.Since(start)) + return nil } diff --git a/plugins/plugin_lifecycle_manager_test.go b/plugins/plugin_lifecycle_manager_test.go index c0621b2a7..800630ce9 100644 --- a/plugins/plugin_lifecycle_manager_test.go +++ b/plugins/plugin_lifecycle_manager_test.go @@ -2,6 +2,7 @@ package plugins import ( "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/plugins/schema" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -22,7 +23,7 @@ var _ = Describe("LifecycleManagement", func() { var lifecycleManager *pluginLifecycleManager BeforeEach(func() { - lifecycleManager = newPluginLifecycleManager() + lifecycleManager = newPluginLifecycleManager(metrics.NewNoopInstance()) }) It("should track initialization state of plugins", func() { @@ -140,5 +141,26 @@ var _ = Describe("LifecycleManagement", func() { Expect(actualKey).To(Equal(expectedKey)) }) + + It("should clear initialization state when requested", func() { + plugin := &plugin{ + ID: "test-plugin", + Capabilities: []string{CapabilityLifecycleManagement}, + Manifest: &schema.PluginManifest{ + Version: "1.0.0", + }, + } + + // Initially not initialized + Expect(lifecycleManager.isInitialized(plugin)).To(BeFalse()) + + // Mark as initialized + lifecycleManager.markInitialized(plugin) + Expect(lifecycleManager.isInitialized(plugin)).To(BeTrue()) + + // Clear initialization state + lifecycleManager.clearInitialized(plugin) + Expect(lifecycleManager.isInitialized(plugin)).To(BeFalse()) + }) }) }) diff --git a/plugins/runtime.go b/plugins/runtime.go index f68175efc..ee298e63d 100644 --- a/plugins/runtime.go +++ b/plugins/runtime.go @@ -41,7 +41,7 @@ var ( // createRuntime returns a function that creates a new wazero runtime and instantiates the required host functions // based on the given plugin permissions -func (m *Manager) createRuntime(pluginID string, permissions schema.PluginManifestPermissions) api.WazeroNewRuntime { +func (m *managerImpl) createRuntime(pluginID string, permissions schema.PluginManifestPermissions) api.WazeroNewRuntime { return func(ctx context.Context) (wazero.Runtime, error) { // Check if runtime already exists if rt, ok := runtimePool.Load(pluginID); ok { @@ -70,7 +70,7 @@ func (m *Manager) createRuntime(pluginID string, permissions schema.PluginManife } // createCachingRuntime handles the complex logic of setting up a new cachingRuntime -func (m *Manager) createCachingRuntime(ctx context.Context, pluginID string, permissions schema.PluginManifestPermissions) (*cachingRuntime, error) { +func (m *managerImpl) createCachingRuntime(ctx context.Context, pluginID string, permissions schema.PluginManifestPermissions) (*cachingRuntime, error) { // Get compilation cache compCache, err := getCompilationCache() if err != nil { @@ -94,7 +94,7 @@ func (m *Manager) createCachingRuntime(ctx context.Context, pluginID string, per } // setupHostServices configures all the permitted host services for a plugin -func (m *Manager) setupHostServices(ctx context.Context, r wazero.Runtime, pluginID string, permissions schema.PluginManifestPermissions) error { +func (m *managerImpl) setupHostServices(ctx context.Context, r wazero.Runtime, pluginID string, permissions schema.PluginManifestPermissions) error { // Define all available host services type hostService struct { name string diff --git a/plugins/runtime_test.go b/plugins/runtime_test.go index 32cd42118..05efe1d1d 100644 --- a/plugins/runtime_test.go +++ b/plugins/runtime_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/plugins/schema" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -34,13 +35,13 @@ var _ = Describe("Runtime", func() { var _ = Describe("CachingRuntime", func() { var ( ctx context.Context - mgr *Manager + mgr *managerImpl plugin *wasmScrobblerPlugin ) BeforeEach(func() { ctx = GinkgoT().Context() - mgr = createManager(nil, nil) + mgr = createManager(nil, metrics.NewNoopInstance()) // Add permissions for the test plugin using typed struct permissions := schema.PluginManifestPermissions{ Http: &schema.PluginManifestPermissionsHttp{ diff --git a/plugins/testdata/fake_init_service/plugin.go b/plugins/testdata/fake_init_service/plugin.go index 5b279b09c..9e6171623 100644 --- a/plugins/testdata/fake_init_service/plugin.go +++ b/plugins/testdata/fake_init_service/plugin.go @@ -4,6 +4,7 @@ package main import ( "context" + "errors" "log" "github.com/navidrome/navidrome/plugins/api" @@ -13,6 +14,22 @@ type initServicePlugin struct{} func (p *initServicePlugin) OnInit(ctx context.Context, req *api.InitRequest) (*api.InitResponse, error) { log.Printf("OnInit called with %v", req) + + // Check for specific error conditions in the config + if req.Config != nil { + if errorType, exists := req.Config["returnError"]; exists { + switch errorType { + case "go_error": + return nil, errors.New("initialization failed with Go error") + case "response_error": + return &api.InitResponse{ + Error: "initialization failed with response error", + }, nil + } + } + } + + // Default: successful initialization return &api.InitResponse{}, nil } diff --git a/plugins/wasm_base_plugin.go b/plugins/wasm_base_plugin.go deleted file mode 100644 index bc1f1d2f5..000000000 --- a/plugins/wasm_base_plugin.go +++ /dev/null @@ -1,119 +0,0 @@ -package plugins - -import ( - "context" - "errors" - "fmt" - "time" - - "github.com/navidrome/navidrome/core/agents" - "github.com/navidrome/navidrome/core/metrics" - "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/model/id" -) - -// newWasmBasePlugin creates a new instance of wasmBasePlugin with the required parameters. -func newWasmBasePlugin[S any, P any](wasmPath, id, capability string, m metrics.Metrics, loader P, loadFunc loaderFunc[S, P]) *wasmBasePlugin[S, P] { - return &wasmBasePlugin[S, P]{ - wasmPath: wasmPath, - id: id, - capability: capability, - loader: loader, - loadFunc: loadFunc, - metrics: m, - } -} - -// LoaderFunc is a generic function type that loads a plugin instance. -type loaderFunc[S any, P any] func(ctx context.Context, loader P, path string) (S, error) - -// wasmBasePlugin is a generic base implementation for WASM plugins. -// S is the service interface type and P is the plugin loader type. -type wasmBasePlugin[S any, P any] struct { - wasmPath string - id string - capability string - loader P - loadFunc loaderFunc[S, P] - metrics metrics.Metrics -} - -func (w *wasmBasePlugin[S, P]) PluginID() string { - return w.id -} - -func (w *wasmBasePlugin[S, P]) Instantiate(ctx context.Context) (any, func(), error) { - return w.getInstance(ctx, "") -} - -func (w *wasmBasePlugin[S, P]) serviceName() string { - return w.id + "_" + w.capability -} - -func (w *wasmBasePlugin[S, P]) getMetrics() metrics.Metrics { - return w.metrics -} - -// getInstance loads a new plugin instance and returns a cleanup function. -func (w *wasmBasePlugin[S, P]) getInstance(ctx context.Context, methodName string) (S, func(), error) { - start := time.Now() - // Add context metadata for tracing - ctx = log.NewContext(ctx, "capability", w.serviceName(), "method", methodName) - - inst, err := w.loadFunc(ctx, w.loader, w.wasmPath) - if err != nil { - var zero S - return zero, func() {}, fmt.Errorf("wasmBasePlugin: failed to load instance for %s: %w", w.serviceName(), err) - } - // Add context metadata for tracing - ctx = log.NewContext(ctx, "instanceID", getInstanceID(inst)) - log.Trace(ctx, "wasmBasePlugin: loaded instance", "elapsed", time.Since(start)) - return inst, func() { - log.Trace(ctx, "wasmBasePlugin: finished using instance", "elapsed", time.Since(start)) - if closer, ok := any(inst).(interface{ Close(context.Context) error }); ok { - _ = closer.Close(ctx) - } - }, nil -} - -type wasmPlugin[S any] interface { - PluginID() string - getInstance(ctx context.Context, methodName string) (S, func(), error) - getMetrics() metrics.Metrics -} - -type errorMapper interface { - mapError(err error) error -} - -func callMethod[S any, R any](ctx context.Context, w wasmPlugin[S], methodName string, fn func(inst S) (R, error)) (R, error) { - // Add a unique call ID to the context for tracing - ctx = log.NewContext(ctx, "callID", id.NewRandom()) - - inst, done, err := w.getInstance(ctx, methodName) - var r R - if err != nil { - return r, err - } - start := time.Now() - defer done() - r, err = fn(inst) - elapsed := time.Since(start) - - if em, ok := any(w).(errorMapper); ok { - mappedErr := em.mapError(err) - - if !errors.Is(mappedErr, agents.ErrNotFound) { - id := w.PluginID() - isOk := mappedErr == nil - metrics := w.getMetrics() - if metrics != nil { - metrics.RecordPluginRequest(ctx, id, methodName, isOk, elapsed.Milliseconds()) - } - log.Trace(ctx, "callMethod", "plugin", id, "method", methodName, "ok", isOk, elapsed) - } - - return r, mappedErr - } - return r, err -} diff --git a/plugins/wasm_base_plugin_test.go b/plugins/wasm_base_plugin_test.go deleted file mode 100644 index 6d6421598..000000000 --- a/plugins/wasm_base_plugin_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package plugins - -import ( - "context" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -type nilInstance struct{} - -var _ = Describe("wasmBasePlugin", func() { - var ctx = context.Background() - - It("should load instance using loadFunc", func() { - called := false - plugin := &wasmBasePlugin[*nilInstance, any]{ - wasmPath: "", - id: "test", - capability: "test", - loadFunc: func(ctx context.Context, _ any, path string) (*nilInstance, error) { - called = true - return &nilInstance{}, nil - }, - } - inst, done, err := plugin.getInstance(ctx, "test") - defer done() - Expect(err).To(BeNil()) - Expect(inst).ToNot(BeNil()) - Expect(called).To(BeTrue()) - }) -}) diff --git a/resources/i18n/eu.json b/resources/i18n/eu.json index 5470ab38b..1adb8b8ba 100644 --- a/resources/i18n/eu.json +++ b/resources/i18n/eu.json @@ -27,23 +27,25 @@ "rating": "Balorazioa", "quality": "Kalitatea", "bpm": "BPM", - "playDate": "Azkenekoz erreproduzitua:", + "playDate": "Azken erreprodukzioa:", "createdAt": "Gehitu zen data:", "grouping": "Multzokatzea", "mood": "Aldartea", "participants": "Partaide gehiago", "tags": "Traola gehiago", "mappedTags": "Esleitutako traolak", - "rawTags": "Traola gordinak" + "rawTags": "Traola gordinak", + "missing": "Ez da aurkitu" }, "actions": { "addToQueue": "Erreproduzitu ondoren", "playNow": "Erreproduzitu orain", "addToPlaylist": "Gehitu erreprodukzio-zerrendara", + "showInPlaylist": "Erakutsi erreprodukzio-zerrendan", "shuffleAll": "Erreprodukzio aleatorioa", "download": "Deskargatu", "playNext": "Hurrengoa", - "info": "Lortu informazioa" + "info": "Erakutsi informazioa" } }, "album": { @@ -61,7 +63,7 @@ "year": "Urtea", "date": "Recording Date", "originalDate": "Jatorrizkoa", - "releaseDate": "Argitaratze-data:", + "releaseDate": "Argitaratze-data", "releases": "Argitaratzea |||| Argitaratzeak", "released": "Argitaratua", "updatedAt": "Aktualizatze-data:", @@ -73,21 +75,22 @@ "releaseType": "Mota", "grouping": "Multzokatzea", "media": "Multimedia", - "mood": "Aldartea" + "mood": "Aldartea", + "missing": "Ez da aurkitu" }, "actions": { "playAll": "Erreproduzitu", - "playNext": "Erreproduzitu segidan", + "playNext": "Erreproduzitu orain", "addToQueue": "Erreproduzitu amaieran", "shuffle": "Aletorioa", "addToPlaylist": "Gehitu zerrendara", "download": "Deskargatu", - "info": "Lortu informazioa", + "info": "Erakutsi informazioa", "share": "Partekatu" }, "lists": { "all": "Guztiak", - "random": "Aleatorioki", + "random": "Aleatorioa", "recentlyAdded": "Berriki gehitutakoak", "recentlyPlayed": "Berriki entzundakoak", "mostPlayed": "Gehien entzundakoak", @@ -105,7 +108,8 @@ "playCount": "Erreprodukzio kopurua", "rating": "Balorazioa", "genre": "Generoa", - "role": "Rola" + "role": "Rola", + "missing": "Ez da aurkitu" }, "roles": { "albumartist": "Albumeko egilea |||| Albumeko artistak", @@ -120,7 +124,13 @@ "mixer": "Nahaslea |||| Nahasleak", "remixer": "Remixerra |||| Remixerrak", "djmixer": "DJ nahaslea |||| DJ nahasleak", - "performer": "Interpretatzailea |||| Interpretatzaileak" + "performer": "Interpretatzailea |||| Interpretatzaileak", + "maincredit": "Albumeko egilea edo egilea |||| Albumeko egileak edo egileak" + }, + "actions": { + "topSongs": "Abesti apartak", + "shuffle": "Aleatorioki", + "radio": "Irratia" } }, "user": { @@ -192,12 +202,19 @@ "selectPlaylist": "Hautatu zerrenda:", "addNewPlaylist": "Sortu \"%{name}\"", "export": "Esportatu", + "saveQueue": "Gorde ilaran daudek erreprodukzio-zerrendan", "makePublic": "Egin publikoa", - "makePrivate": "Egin pribatua" + "makePrivate": "Egin pribatua", + "searchOrCreate": "Bilatu erreprodukzio-zerrenda edo idatzi berria sortzeko…", + "pressEnterToCreate": "Sakatu Enter erreprodukzio-zerrenda berria sortzeko", + "removeFromSelection": "Kendu hautaketatik", + "removeSymbol": "×" }, "message": { "duplicate_song": "Hautatutako abesti batzuk lehendik ere daude zerrendan", - "song_exist": "Bikoiztutakoak gehitzen ari dira erreprodukzio-zerrendara. Ziur gehitu nahi dituzula?" + "song_exist": "Bikoiztutakoak gehitzen ari dira erreprodukzio-zerrendara. Ziur gehitu nahi dituzula?", + "noPlaylistsFound": "Ez da erreprodukzio-zerrenda aurkitu", + "noPlaylists": "Ez dago erreprodukzio-zerrendarik eskuragarri" } }, "radio": { @@ -233,7 +250,7 @@ "actions": {} }, "missing": { - "name": "Fitxategia falta da|||| Fitxategiak falta dira", + "name": "Aurkitu ez den fitxategia |||| Aurkitu ez diren fitxategiak", "empty": "Ez da fitxategirik falta", "fields": { "path": "Bidea", @@ -242,10 +259,10 @@ }, "actions": { "remove": "Kendu", - "remove_all": "Kendu guztia" + "remove_all": "Kendu guztiak" }, "notifications": { - "removed": "Faltan zeuden fitxategiak kendu dira" + "removed": "Aurkitzen ez ziren fitxategiak kendu dira" } } }, @@ -399,6 +416,8 @@ "transcodingDisabled": "Segurtasun arrazoiak direla-eta, transkodeketaren ezarpenak web-interfazearen bidez aldatzea ezgaituta dago. Transkodeketa-aukerak aldatu (editatu edo gehitu) nahi badituzu, berrabiarazi zerbitzaria konfigurazio-aukeraren %{config}-arekin.", "transcodingEnabled": "Navidrome %{config}-ekin martxan dago eta, beraz, web-interfazeko transkodeketa-ataletik sistema-komandoak exekuta daitezke. Segurtasun arrazoiak tarteko, ezgaitzea gomendatzen dugu, eta transkodeketa-aukerak konfiguratzen ari zarenean bakarrik gaitzea.", "songsAddedToPlaylist": "Abesti bat zerrendara gehitu da |||| %{smart_count} abesti zerrendara gehitu dira", + "noSimilarSongsFound": "Ez da antzeko abestirik aurkitu", + "noTopSongsFound": "Ez da aparteko abestirik aurkitu", "noPlaylistsAvailable": "Ez dago zerrendarik erabilgarri", "delete_user_title": "Ezabatu '%{name}' erabiltzailea", "delete_user_content": "Ziur zaide erabiltzaile hau eta bere datu guztiak (zerrendak eta hobespenak barne) ezabatu nahi dituzula?", @@ -480,8 +499,8 @@ "playModeText": { "order": "Ordenean", "orderLoop": "Errepikatu", - "singleLoop": "Errepikatu bakarra", - "shufflePlay": "Aleatorioa" + "singleLoop": "Errepikatu abesti hau", + "shufflePlay": "Aleatorioki" } }, "about": { @@ -494,6 +513,21 @@ "disabled": "Ezgaituta", "waiting": "Zain" } + }, + "tabs": { + "about": "Honi buruz", + "config": "Konfigurazioa" + }, + "config": { + "configName": "Konfigurazioaren izena", + "environmentVariable": "Ingurune-aldagaia", + "currentValue": "Uneko balioa", + "configurationFile": "Konfigurazio-fitxategia", + "exportToml": "Esportatu konfigurazioa (TOML)", + "exportSuccess": "Konfigurazioa arbelera esportatu da TOML formatuan", + "exportFailed": "Konfigurazioa kopiatzeak huts egin du", + "devFlagsHeader": "Garapen-adierazleak (aldatu/kendu litezke)", + "devFlagsComment": "Ezarpen esperimentalak dira eta litekeena da etorkizunean desagertzea" } }, "activity": { @@ -507,6 +541,11 @@ "status": "Errorea arakatzean", "elapsedTime": "Igarotako denbora" }, + "nowPlaying": { + "title": "Une honetan erreproduzitzen", + "empty": "Ez dago erreproduzitzeko ezer", + "minutesAgo": "Duela minutu %{smart_count} |||| Duela %{smart_count} minutu" + }, "help": { "title": "Navidromeren laster-teklak", "hotkeys": { diff --git a/resources/i18n/hu.json b/resources/i18n/hu.json index 8eb1a04f1..23a3cc6b5 100644 --- a/resources/i18n/hu.json +++ b/resources/i18n/hu.json @@ -41,6 +41,7 @@ "addToQueue": "Lejátszás útolsóként", "playNow": "Lejátszás", "addToPlaylist": "Lejátszási listához adás", + "showInPlaylist": "Megjelenítés a lejátszási listában", "shuffleAll": "Keverés", "download": "Letöltés", "playNext": "Lejátszás következőként", @@ -123,7 +124,13 @@ "mixer": "Keverő |||| Keverők", "remixer": "Átdolgozó |||| Átdolgozók", "djmixer": "DJ keverő |||| DJ keverők", - "performer": "Előadóművész |||| Előadóművészek" + "performer": "Előadóművész |||| Előadóművészek", + "maincredit": "Album előadó vagy előadó |||| Album előadók vagy előadók" + }, + "actions": { + "topSongs": "Top számok", + "shuffle": "Keverés", + "radio": "Rádió" } }, "user": { @@ -197,11 +204,17 @@ "export": "Exportálás", "saveQueue": "Műsorlista elmentése lejátszási listaként", "makePublic": "Publikussá tétel", - "makePrivate": "Priváttá tétel" + "makePrivate": "Priváttá tétel", + "searchOrCreate": "Keress lejátszási listák között vagy hozz létre egyet...", + "pressEnterToCreate": "Nyomj Entert, hogy létrehozz egy lejátszási listát", + "removeFromSelection": "Eltávolítás a kiválasztásból", + "removeSymbol": "×" }, "message": { "duplicate_song": "Duplikált számok hozzáadása", - "song_exist": "Egyes számok már hozzá vannak adva a listához. Még egyszer hozzá akarod adni?" + "song_exist": "Egyes számok már hozzá vannak adva a listához. Még egyszer hozzá akarod adni?", + "noPlaylistsFound": "Nem található lejátszási lista", + "noPlaylists": "Nincsenek lejátszási listák" } }, "radio": { @@ -401,6 +414,8 @@ "transcodingDisabled": "Az átkódolási konfiguráció módosítása a webes felületen keresztül biztonsági okokból nem lehetséges. Ha módosítani szeretnéd az átkódolási beállításokat, indítsd újra a kiszolgálót a %{config} konfigurációs opcióval.", "transcodingEnabled": "A Navidrome jelenleg a következőkkel fut %{config}, ez lehetővé teszi a rendszerparancsok futtatását az átkódolási beállításokból a webes felület segítségével. Javasoljuk, hogy biztonsági okokból tiltsd ezt le, és csak az átkódolási beállítások konfigurálásának idejére kapcsold be.", "songsAddedToPlaylist": "1 szám hozzáadva a lejátszási listához |||| %{smart_count} szám hozzáadva a lejátszási listához", + "noSimilarSongsFound": "Nem találhatóak hasonló számok", + "noTopSongsFound": "Nincsenek top számok", "noPlaylistsAvailable": "Nem áll rendelkezésre", "delete_user_title": "Felhasználó törlése '%{name}'", "delete_user_content": "Biztos, hogy törölni akarod ezt a felhasználót az adataival (beállítások és lejátszási listák) együtt?", @@ -496,6 +511,21 @@ "disabled": "Kikapcsolva", "waiting": "Várakozás" } + }, + "tabs": { + "about": "Rólunk", + "config": "Konfiguráció" + }, + "config": { + "configName": "Beállítás neve", + "environmentVariable": "Környezeti változó", + "currentValue": "Jelenlegi érték", + "configurationFile": "Konfigurációs fájl", + "exportToml": "Konfiguráció exportálása (TOML)", + "exportSuccess": "Konfiguráció kiexportálva a vágólapra, TOML formában", + "exportFailed": "Nem sikerült kimásolni a konfigurációt", + "devFlagsHeader": "Fejlesztői beállítások (változások/eltávolítás jogát fenntartjuk)", + "devFlagsComment": "Ezek kísérleti beállítások, és a jövőbeli verziókban eltávolíthatók" } }, "activity": { @@ -509,6 +539,11 @@ "status": "Szkennelési hiba", "elapsedTime": "Eltelt idő" }, + "nowPlaying": { + "title": "Most megy", + "empty": "Nem hallgatsz semmit", + "minutesAgo": "%{smart_count} perce |||| %{smart_count} perce" + }, "help": { "title": "Navidrome Gyorsbillentyűk", "hotkeys": { diff --git a/resources/mappings.yaml b/resources/mappings.yaml index f461d889e..d1da5c620 100644 --- a/resources/mappings.yaml +++ b/resources/mappings.yaml @@ -108,7 +108,8 @@ main: bpm: aliases: [ tbpm, bpm, tmpo, wm/beatsperminute ] lyrics: - aliases: [ uslt:description, lyrics, ©lyr, wm/lyrics, unsyncedlyrics ] + # Note, @lyr and wm/lyrics have been removed. Taglib somehow appears to always populate `lyrics:xxx` + aliases: [ uslt:description, lyrics, unsyncedlyrics ] maxLength: 32768 type: pair # ex: lyrics:eng, lyrics:xxx comment: diff --git a/tests/fixtures/mixed-lyrics.flac b/tests/fixtures/mixed-lyrics.flac new file mode 100644 index 000000000..d048234f5 Binary files /dev/null and b/tests/fixtures/mixed-lyrics.flac differ diff --git a/ui/src/artist/ArtistActions.test.jsx b/ui/src/artist/ArtistActions.test.jsx index 90be28409..a11ee50e3 100644 --- a/ui/src/artist/ArtistActions.test.jsx +++ b/ui/src/artist/ArtistActions.test.jsx @@ -49,11 +49,21 @@ describe('ArtistActions', () => { // Mock console.error to suppress error logging in tests vi.spyOn(console, 'error').mockImplementation(() => {}) + const songWithReplayGain = { + id: 'rec1', + replayGain: { + albumGain: -5, + albumPeak: 1, + trackGain: -6, + trackPeak: 0.8, + }, + } + subsonic.getSimilarSongs2.mockResolvedValue({ json: { 'subsonic-response': { status: 'ok', - similarSongs2: { song: [{ id: 'rec1' }] }, + similarSongs2: { song: [songWithReplayGain] }, }, }, }) @@ -61,7 +71,7 @@ describe('ArtistActions', () => { json: { 'subsonic-response': { status: 'ok', - topSongs: { song: [{ id: 'rec1' }] }, + topSongs: { song: [songWithReplayGain] }, }, }, }) @@ -93,6 +103,22 @@ describe('ArtistActions', () => { ) expect(mockDispatch).toHaveBeenCalled() }) + + it('maps replaygain info', async () => { + renderArtistActions() + clickActionButton('radio') + + await waitFor(() => + expect(subsonic.getSimilarSongs2).toHaveBeenCalledWith('ar1', 100), + ) + const action = mockDispatch.mock.calls[0][0] + expect(action.data.rec1).toMatchObject({ + rgAlbumGain: -5, + rgAlbumPeak: 1, + rgTrackGain: -6, + rgTrackPeak: 0.8, + }) + }) }) describe('Play action', () => { @@ -106,6 +132,22 @@ describe('ArtistActions', () => { expect(mockDispatch).toHaveBeenCalled() }) + it('maps replaygain info for top songs', async () => { + renderArtistActions() + clickActionButton('topSongs') + + await waitFor(() => + expect(subsonic.getTopSongs).toHaveBeenCalledWith('Artist', 100), + ) + const action = mockDispatch.mock.calls[0][0] + expect(action.data.rec1).toMatchObject({ + rgAlbumGain: -5, + rgAlbumPeak: 1, + rgTrackGain: -6, + rgTrackPeak: 0.8, + }) + }) + it('handles API rejection', async () => { subsonic.getTopSongs.mockRejectedValue(new Error('Network error')) diff --git a/ui/src/artist/actions.js b/ui/src/artist/actions.js index 0ab648fa0..6a8fbd9c6 100644 --- a/ui/src/artist/actions.js +++ b/ui/src/artist/actions.js @@ -1,6 +1,32 @@ import subsonic from '../subsonic/index.js' import { playTracks } from '../actions/index.js' +const mapReplayGain = (song) => { + const { replayGain: rg } = song + if (!rg) { + return song + } + + return { + ...song, + ...(rg.albumGain !== undefined && { rgAlbumGain: rg.albumGain }), + ...(rg.albumPeak !== undefined && { rgAlbumPeak: rg.albumPeak }), + ...(rg.trackGain !== undefined && { rgTrackGain: rg.trackGain }), + ...(rg.trackPeak !== undefined && { rgTrackPeak: rg.trackPeak }), + } +} + +const processSongsForPlayback = (songs) => { + const songData = {} + const ids = [] + songs.forEach((s) => { + const song = mapReplayGain(s) + songData[song.id] = song + ids.push(song.id) + }) + return { songData, ids } +} + export const playTopSongs = async (dispatch, notify, artistName) => { const res = await subsonic.getTopSongs(artistName, 100) const data = res.json['subsonic-response'] @@ -17,12 +43,7 @@ export const playTopSongs = async (dispatch, notify, artistName) => { return } - const songData = {} - const ids = [] - songs.forEach((s) => { - songData[s.id] = s - ids.push(s.id) - }) + const { songData, ids } = processSongsForPlayback(songs) dispatch(playTracks(songData, ids)) } @@ -42,12 +63,7 @@ export const playSimilar = async (dispatch, notify, id) => { return } - const songData = {} - const ids = [] - songs.forEach((s) => { - songData[s.id] = s - ids.push(s.id) - }) + const { songData, ids } = processSongsForPlayback(songs) dispatch(playTracks(songData, ids)) } diff --git a/ui/src/eventStream.test.js b/ui/src/eventStream.test.js index 77d061c19..5bd0dd0be 100644 --- a/ui/src/eventStream.test.js +++ b/ui/src/eventStream.test.js @@ -32,6 +32,8 @@ describe('startEventStream', () => { localStorage.setItem('is-authenticated', 'true') localStorage.setItem('token', 'abc') config.devNewEventStream = true + // Mock console.log to suppress output during tests + vi.spyOn(console, 'log').mockImplementation(() => {}) }) afterEach(() => {