From 69e7d163fc00d8a723ac6f3072c6fdcbfffff1c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 15 Mar 2026 13:18:54 -0400 Subject: [PATCH 1/5] remove built-in Spotify integration (#5197) * refactor: remove built-in Spotify integration Remove the Spotify adapter and all related configuration, replacing the built-in integration with the plugin system. This deletes the adapters/spotify package, removes Spotify config options (ID/Secret), updates the default agents list from "deezer,lastfm,spotify" to "deezer,lastfm", and cleans up all references across configuration, metrics, logging, artwork caching, and documentation. Users with Spotify config options will now see a warning that the options are no longer available. * feat: add ListenBrainz to list of default agents Signed-off-by: Deluan --------- Signed-off-by: Deluan --- adapters/spotify/client.go | 116 --------------------- adapters/spotify/client_test.go | 131 ------------------------ adapters/spotify/responses.go | 30 ------ adapters/spotify/responses_test.go | 48 --------- adapters/spotify/spotify.go | 96 ----------------- adapters/spotify/spotify_suite_test.go | 17 --- cmd/root.go | 1 - cmd/wire_gen.go | 1 - conf/configuration.go | 32 ++++-- core/agents/README.md | 2 +- core/artwork/reader_artist.go | 2 +- core/external/provider_topsongs_test.go | 1 - core/metrics/insights.go | 1 - core/metrics/insights/data.go | 1 - log/log.go | 1 - plugins/examples/wikimedia/README.md | 2 +- plugins/manifest-schema.json | 4 +- plugins/manifest_gen.go | 4 +- plugins/manifest_test.go | 4 +- server/initial_setup.go | 6 -- server/nativeapi/config.go | 4 +- server/nativeapi/config_test.go | 8 -- 22 files changed, 32 insertions(+), 480 deletions(-) delete mode 100644 adapters/spotify/client.go delete mode 100644 adapters/spotify/client_test.go delete mode 100644 adapters/spotify/responses.go delete mode 100644 adapters/spotify/responses_test.go delete mode 100644 adapters/spotify/spotify.go delete mode 100644 adapters/spotify/spotify_suite_test.go diff --git a/adapters/spotify/client.go b/adapters/spotify/client.go deleted file mode 100644 index 975175930..000000000 --- a/adapters/spotify/client.go +++ /dev/null @@ -1,116 +0,0 @@ -package spotify - -import ( - "context" - "encoding/base64" - "encoding/json" - "errors" - "fmt" - "io" - "net/http" - "net/url" - "strconv" - "strings" - - "github.com/navidrome/navidrome/log" -) - -const apiBaseUrl = "https://api.spotify.com/v1/" - -var ( - ErrNotFound = errors.New("spotify: not found") -) - -type httpDoer interface { - Do(req *http.Request) (*http.Response, error) -} - -func newClient(id, secret string, hc httpDoer) *client { - return &client{id, secret, hc} -} - -type client struct { - id string - secret string - hc httpDoer -} - -func (c *client) searchArtists(ctx context.Context, name string, limit int) ([]Artist, error) { - token, err := c.authorize(ctx) - if err != nil { - return nil, err - } - - params := url.Values{} - params.Add("type", "artist") - params.Add("q", name) - params.Add("offset", "0") - params.Add("limit", strconv.Itoa(limit)) - req, _ := http.NewRequestWithContext(ctx, "GET", apiBaseUrl+"search", nil) - req.URL.RawQuery = params.Encode() - req.Header.Add("Authorization", "Bearer "+token) - - var results SearchResults - err = c.makeRequest(req, &results) - if err != nil { - return nil, err - } - - if len(results.Artists.Items) == 0 { - return nil, ErrNotFound - } - return results.Artists.Items, err -} - -func (c *client) authorize(ctx context.Context) (string, error) { - payload := url.Values{} - payload.Add("grant_type", "client_credentials") - - encodePayload := payload.Encode() - req, _ := http.NewRequestWithContext(ctx, "POST", "https://accounts.spotify.com/api/token", strings.NewReader(encodePayload)) - req.Header.Add("Content-Type", "application/x-www-form-urlencoded") - req.Header.Add("Content-Length", strconv.Itoa(len(encodePayload))) - auth := c.id + ":" + c.secret - req.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(auth))) - - response := map[string]any{} - err := c.makeRequest(req, &response) - if err != nil { - return "", err - } - - if v, ok := response["access_token"]; ok { - return v.(string), nil - } - log.Error(ctx, "Invalid spotify response", "resp", response) - return "", errors.New("invalid response") -} - -func (c *client) makeRequest(req *http.Request, response any) error { - log.Trace(req.Context(), fmt.Sprintf("Sending Spotify %s request", req.Method), "url", req.URL) - resp, err := c.hc.Do(req) - if err != nil { - return err - } - - defer resp.Body.Close() - data, err := io.ReadAll(resp.Body) - if err != nil { - return err - } - - if resp.StatusCode != 200 { - return c.parseError(data) - } - - return json.Unmarshal(data, response) -} - -func (c *client) parseError(data []byte) error { - var e Error - err := json.Unmarshal(data, &e) - if err != nil { - return err - } - return fmt.Errorf("spotify error(%s): %s", e.Code, e.Message) -} diff --git a/adapters/spotify/client_test.go b/adapters/spotify/client_test.go deleted file mode 100644 index 2782d2122..000000000 --- a/adapters/spotify/client_test.go +++ /dev/null @@ -1,131 +0,0 @@ -package spotify - -import ( - "bytes" - "context" - "io" - "net/http" - "os" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("client", func() { - var httpClient *fakeHttpClient - var client *client - - BeforeEach(func() { - httpClient = &fakeHttpClient{} - client = newClient("SPOTIFY_ID", "SPOTIFY_SECRET", httpClient) - }) - - Describe("ArtistImages", func() { - It("returns artist images from a successful request", func() { - f, _ := os.Open("tests/fixtures/spotify.search.artist.json") - httpClient.mock("https://api.spotify.com/v1/search", http.Response{Body: f, StatusCode: 200}) - httpClient.mock("https://accounts.spotify.com/api/token", http.Response{ - StatusCode: 200, - Body: io.NopCloser(bytes.NewBufferString(`{"access_token": "NEW_ACCESS_TOKEN","token_type": "Bearer","expires_in": 3600}`)), - }) - - artists, err := client.searchArtists(context.TODO(), "U2", 10) - Expect(err).To(BeNil()) - Expect(artists).To(HaveLen(20)) - Expect(artists[0].Popularity).To(Equal(82)) - - images := artists[0].Images - Expect(images).To(HaveLen(3)) - Expect(images[0].Width).To(Equal(640)) - Expect(images[1].Width).To(Equal(320)) - Expect(images[2].Width).To(Equal(160)) - }) - - It("fails if artist was not found", func() { - httpClient.mock("https://api.spotify.com/v1/search", http.Response{ - StatusCode: 200, - Body: io.NopCloser(bytes.NewBufferString(`{ - "artists" : { - "href" : "https://api.spotify.com/v1/search?query=dasdasdas%2Cdna&type=artist&offset=0&limit=20", - "items" : [ ], "limit" : 20, "next" : null, "offset" : 0, "previous" : null, "total" : 0 - }}`)), - }) - httpClient.mock("https://accounts.spotify.com/api/token", http.Response{ - StatusCode: 200, - Body: io.NopCloser(bytes.NewBufferString(`{"access_token": "NEW_ACCESS_TOKEN","token_type": "Bearer","expires_in": 3600}`)), - }) - - _, err := client.searchArtists(context.TODO(), "U2", 10) - Expect(err).To(MatchError(ErrNotFound)) - }) - - It("fails if not able to authorize", func() { - f, _ := os.Open("tests/fixtures/spotify.search.artist.json") - httpClient.mock("https://api.spotify.com/v1/search", http.Response{Body: f, StatusCode: 200}) - httpClient.mock("https://accounts.spotify.com/api/token", http.Response{ - StatusCode: 400, - Body: io.NopCloser(bytes.NewBufferString(`{"error":"invalid_client","error_description":"Invalid client"}`)), - }) - - _, err := client.searchArtists(context.TODO(), "U2", 10) - Expect(err).To(MatchError("spotify error(invalid_client): Invalid client")) - }) - }) - - Describe("authorize", func() { - It("returns an access_token on successful authorization", func() { - httpClient.mock("https://accounts.spotify.com/api/token", http.Response{ - StatusCode: 200, - Body: io.NopCloser(bytes.NewBufferString(`{"access_token": "NEW_ACCESS_TOKEN","token_type": "Bearer","expires_in": 3600}`)), - }) - - token, err := client.authorize(context.TODO()) - Expect(err).To(BeNil()) - Expect(token).To(Equal("NEW_ACCESS_TOKEN")) - auth := httpClient.lastRequest.Header.Get("Authorization") - Expect(auth).To(Equal("Basic U1BPVElGWV9JRDpTUE9USUZZX1NFQ1JFVA==")) - }) - - It("fails on unsuccessful authorization", func() { - httpClient.mock("https://accounts.spotify.com/api/token", http.Response{ - StatusCode: 400, - Body: io.NopCloser(bytes.NewBufferString(`{"error":"invalid_client","error_description":"Invalid client"}`)), - }) - - _, err := client.authorize(context.TODO()) - Expect(err).To(MatchError("spotify error(invalid_client): Invalid client")) - }) - - It("fails on invalid JSON response", func() { - httpClient.mock("https://accounts.spotify.com/api/token", http.Response{ - StatusCode: 200, - Body: io.NopCloser(bytes.NewBufferString(`{NOT_VALID}`)), - }) - - _, err := client.authorize(context.TODO()) - Expect(err).To(MatchError("invalid character 'N' looking for beginning of object key string")) - }) - }) -}) - -type fakeHttpClient struct { - responses map[string]*http.Response - lastRequest *http.Request -} - -func (c *fakeHttpClient) mock(url string, response http.Response) { - if c.responses == nil { - c.responses = make(map[string]*http.Response) - } - c.responses[url] = &response -} - -func (c *fakeHttpClient) Do(req *http.Request) (*http.Response, error) { - c.lastRequest = req - u := req.URL - u.RawQuery = "" - if resp, ok := c.responses[u.String()]; ok { - return resp, nil - } - panic("URL not mocked: " + u.String()) -} diff --git a/adapters/spotify/responses.go b/adapters/spotify/responses.go deleted file mode 100644 index 21166bf74..000000000 --- a/adapters/spotify/responses.go +++ /dev/null @@ -1,30 +0,0 @@ -package spotify - -type SearchResults struct { - Artists ArtistsResult `json:"artists"` -} - -type ArtistsResult struct { - HRef string `json:"href"` - Items []Artist `json:"items"` -} - -type Artist struct { - Genres []string `json:"genres"` - HRef string `json:"href"` - ID string `json:"id"` - Popularity int `json:"popularity"` - Images []Image `json:"images"` - Name string `json:"name"` -} - -type Image struct { - URL string `json:"url"` - Width int `json:"width"` - Height int `json:"height"` -} - -type Error struct { - Code string `json:"error"` - Message string `json:"error_description"` -} diff --git a/adapters/spotify/responses_test.go b/adapters/spotify/responses_test.go deleted file mode 100644 index 704119816..000000000 --- a/adapters/spotify/responses_test.go +++ /dev/null @@ -1,48 +0,0 @@ -package spotify - -import ( - "encoding/json" - "os" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Responses", func() { - Describe("Search type=artist", func() { - It("parses the artist search result correctly ", func() { - var resp SearchResults - body, _ := os.ReadFile("tests/fixtures/spotify.search.artist.json") - err := json.Unmarshal(body, &resp) - Expect(err).To(BeNil()) - - Expect(resp.Artists.Items).To(HaveLen(20)) - u2 := resp.Artists.Items[0] - Expect(u2.Name).To(Equal("U2")) - Expect(u2.Genres).To(ContainElements("irish rock", "permanent wave", "rock")) - Expect(u2.ID).To(Equal("51Blml2LZPmy7TTiAg47vQ")) - Expect(u2.HRef).To(Equal("https://api.spotify.com/v1/artists/51Blml2LZPmy7TTiAg47vQ")) - Expect(u2.Images[0].URL).To(Equal("https://i.scdn.co/image/e22d5c0c8139b8439440a69854ed66efae91112d")) - Expect(u2.Images[0].Width).To(Equal(640)) - Expect(u2.Images[0].Height).To(Equal(640)) - Expect(u2.Images[1].URL).To(Equal("https://i.scdn.co/image/40d6c5c14355cfc127b70da221233315497ec91d")) - Expect(u2.Images[1].Width).To(Equal(320)) - Expect(u2.Images[1].Height).To(Equal(320)) - Expect(u2.Images[2].URL).To(Equal("https://i.scdn.co/image/7293d6752ae8a64e34adee5086858e408185b534")) - Expect(u2.Images[2].Width).To(Equal(160)) - Expect(u2.Images[2].Height).To(Equal(160)) - }) - }) - - Describe("Error", func() { - It("parses the error response correctly", func() { - var errorResp Error - body := []byte(`{"error":"invalid_client","error_description":"Invalid client"}`) - err := json.Unmarshal(body, &errorResp) - Expect(err).To(BeNil()) - - Expect(errorResp.Code).To(Equal("invalid_client")) - Expect(errorResp.Message).To(Equal("Invalid client")) - }) - }) -}) diff --git a/adapters/spotify/spotify.go b/adapters/spotify/spotify.go deleted file mode 100644 index 633c32984..000000000 --- a/adapters/spotify/spotify.go +++ /dev/null @@ -1,96 +0,0 @@ -package spotify - -import ( - "context" - "errors" - "fmt" - "net/http" - "sort" - "strings" - - "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/consts" - "github.com/navidrome/navidrome/core/agents" - "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/utils/cache" - "github.com/xrash/smetrics" -) - -const spotifyAgentName = "spotify" - -type spotifyAgent struct { - ds model.DataStore - id string - secret string - client *client -} - -func spotifyConstructor(ds model.DataStore) agents.Interface { - if conf.Server.Spotify.ID == "" || conf.Server.Spotify.Secret == "" { - return nil - } - l := &spotifyAgent{ - ds: ds, - id: conf.Server.Spotify.ID, - secret: conf.Server.Spotify.Secret, - } - hc := &http.Client{ - Timeout: consts.DefaultHttpClientTimeOut, - } - chc := cache.NewHTTPClient(hc, consts.DefaultHttpClientTimeOut) - l.client = newClient(l.id, l.secret, chc) - return l -} - -func (s *spotifyAgent) AgentName() string { - return spotifyAgentName -} - -func (s *spotifyAgent) GetArtistImages(ctx context.Context, id, name, mbid string) ([]agents.ExternalImage, error) { - a, err := s.searchArtist(ctx, name) - if err != nil { - if errors.Is(err, model.ErrNotFound) { - log.Warn(ctx, "Artist not found in Spotify", "artist", name) - } else { - log.Error(ctx, "Error calling Spotify", "artist", name, err) - } - return nil, err - } - - var res []agents.ExternalImage - for _, img := range a.Images { - res = append(res, agents.ExternalImage{ - URL: img.URL, - Size: img.Width, - }) - } - return res, nil -} - -func (s *spotifyAgent) searchArtist(ctx context.Context, name string) (*Artist, error) { - artists, err := s.client.searchArtists(ctx, name, 40) - if err != nil || len(artists) == 0 { - return nil, model.ErrNotFound - } - name = strings.ToLower(name) - - // Sort results, prioritizing artists with images, with similar names and with high popularity, in this order - sort.Slice(artists, func(i, j int) bool { - ai := fmt.Sprintf("%-5t-%03d-%04d", len(artists[i].Images) == 0, smetrics.WagnerFischer(name, strings.ToLower(artists[i].Name), 1, 1, 2), 1000-artists[i].Popularity) - aj := fmt.Sprintf("%-5t-%03d-%04d", len(artists[j].Images) == 0, smetrics.WagnerFischer(name, strings.ToLower(artists[j].Name), 1, 1, 2), 1000-artists[j].Popularity) - return ai < aj - }) - - // If the first one has the same name, that's the one - if strings.ToLower(artists[0].Name) != name { - return nil, model.ErrNotFound - } - return &artists[0], err -} - -func init() { - conf.AddHook(func() { - agents.Register(spotifyAgentName, spotifyConstructor) - }) -} diff --git a/adapters/spotify/spotify_suite_test.go b/adapters/spotify/spotify_suite_test.go deleted file mode 100644 index 275b05e73..000000000 --- a/adapters/spotify/spotify_suite_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package spotify - -import ( - "testing" - - "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/tests" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -func TestSpotify(t *testing.T) { - tests.Init(t, false) - log.SetLevel(log.LevelFatal) - RegisterFailHandler(Fail) - RunSpecs(t, "Spotify Test Suite") -} diff --git a/cmd/root.go b/cmd/root.go index ff9a574ee..5fdb591ff 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -27,7 +27,6 @@ import ( _ "github.com/navidrome/navidrome/adapters/gotaglib" _ "github.com/navidrome/navidrome/adapters/lastfm" _ "github.com/navidrome/navidrome/adapters/listenbrainz" - _ "github.com/navidrome/navidrome/adapters/spotify" _ "github.com/navidrome/navidrome/adapters/taglib" ) diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index d045c12ec..c2f6edaf8 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -39,7 +39,6 @@ import ( _ "github.com/navidrome/navidrome/adapters/gotaglib" _ "github.com/navidrome/navidrome/adapters/lastfm" _ "github.com/navidrome/navidrome/adapters/listenbrainz" - _ "github.com/navidrome/navidrome/adapters/spotify" _ "github.com/navidrome/navidrome/adapters/taglib" ) diff --git a/conf/configuration.go b/conf/configuration.go index dc5fd1b59..986ab7a5a 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -105,7 +105,6 @@ type configOptions struct { Inspect inspectOptions `json:",omitzero"` Subsonic subsonicOptions `json:",omitzero"` LastFM lastfmOptions `json:",omitzero"` - Spotify spotifyOptions `json:",omitzero"` Deezer deezerOptions `json:",omitzero"` ListenBrainz listenBrainzOptions `json:",omitzero"` EnableScrobbleHistory bool @@ -187,11 +186,6 @@ type lastfmOptions struct { Languages []string // Computed from Language, split by comma } -type spotifyOptions struct { - ID string - Secret string //nolint:gosec -} - type deezerOptions struct { Enabled bool Language string @@ -411,6 +405,7 @@ func Load(noConfigDump bool) { // Parse Deezer.Language into Languages slice (comma-separated, with fallback to DefaultInfoLanguage) Server.Deezer.Languages = parseLanguages(Server.Deezer.Language) + // Deprecated options logDeprecatedOptions("Scanner.GenreSeparators", "") logDeprecatedOptions("Scanner.GroupAlbumReleases", "") logDeprecatedOptions("DevEnableBufferedScrobble", "") // Deprecated: Buffered scrobbling is now always enabled and this option is ignored @@ -420,6 +415,9 @@ func Load(noConfigDump bool) { logDeprecatedOptions("HTTPSecurityHeaders.CustomFrameOptionsValue", "HTTPHeaders.FrameOptions") logDeprecatedOptions("CoverJpegQuality", "CoverArtQuality") + // Removed options + logRemovedOptions("Spotify.ID", "Spotify.Secret") + // Call init hooks for _, hook := range hooks { hook() @@ -444,6 +442,23 @@ func logDeprecatedOptions(oldName, newName string) { } } +// logRemovedOptions checks if the option is set, and if yes, outputs a warning message saying the option is +// not available anymore +func logRemovedOptions(options ...string) { + for _, option := range options { + envVar := "ND_" + strings.ToUpper(strings.ReplaceAll(option, ".", "_")) + logWarning := func(option string) { + log.Warn(fmt.Sprintf("Option '%s' is not available anymore and will be ignored. Please remove it from your config", option)) + } + if viper.InConfig(option) { + logWarning(option) + } + if os.Getenv(envVar) != "" { + logWarning(envVar) + } + } +} + // mapDeprecatedOption is used to provide backwards compatibility for deprecated options. It should be called after // the config has been read by viper, but before unmarshalling it into the Config struct. func mapDeprecatedOption(legacyName, newName string) { @@ -482,7 +497,6 @@ func disableExternalServices() { Server.EnableInsightsCollector = false Server.EnableM3UExternalAlbumArt = false Server.LastFM.Enabled = false - Server.Spotify.ID = "" Server.Deezer.Enabled = false Server.ListenBrainz.Enabled = false Server.Agents = "" @@ -712,14 +726,12 @@ func setViperDefaults() { viper.SetDefault("subsonic.enableaveragerating", true) viper.SetDefault("subsonic.legacyclients", "DSub") viper.SetDefault("subsonic.minimalclients", "SubMusic") - viper.SetDefault("agents", "deezer,lastfm,spotify") + viper.SetDefault("agents", "deezer,lastfm,listenbrainz") viper.SetDefault("lastfm.enabled", true) viper.SetDefault("lastfm.language", consts.DefaultInfoLanguage) viper.SetDefault("lastfm.apikey", "") viper.SetDefault("lastfm.secret", "") viper.SetDefault("lastfm.scrobblefirstartistonly", false) - viper.SetDefault("spotify.id", "") - viper.SetDefault("spotify.secret", "") viper.SetDefault("deezer.enabled", true) viper.SetDefault("deezer.language", consts.DefaultInfoLanguage) viper.SetDefault("listenbrainz.enabled", true) diff --git a/core/agents/README.md b/core/agents/README.md index 1a3a8e96e..cce62889c 100644 --- a/core/agents/README.md +++ b/core/agents/README.md @@ -7,6 +7,6 @@ A new agent must comply with these simple implementation rules: 2) Implement one or more of the `*Retriever()` interfaces. That's where the agent's logic resides. 3) Register itself (in its `init()` function). -For an agent to be used it needs to be listed in the `Agents` config option (default is `"lastfm,spotify"`). The order dictates the priority of the agents +For an agent to be used it needs to be listed in the `Agents` config option (default is `"deezer,lastfm"`). The order dictates the priority of the agents For a simple Agent example, look at the [local_agent](local_agent.go) agent source code. diff --git a/core/artwork/reader_artist.go b/core/artwork/reader_artist.go index 9905039be..990942d87 100644 --- a/core/artwork/reader_artist.go +++ b/core/artwork/reader_artist.go @@ -79,7 +79,7 @@ func newArtistArtworkReader(ctx context.Context, artwork *artwork, artID model.A } func (a *artistReader) Key() string { - hash := md5.Sum([]byte(conf.Server.Agents + conf.Server.Spotify.ID)) + hash := md5.Sum([]byte(conf.Server.Agents)) return fmt.Sprintf( "%s.%t.%x", a.cacheKey.Key(), diff --git a/core/external/provider_topsongs_test.go b/core/external/provider_topsongs_test.go index 0e513aa37..b73c8ab3e 100644 --- a/core/external/provider_topsongs_test.go +++ b/core/external/provider_topsongs_test.go @@ -6,7 +6,6 @@ import ( _ "github.com/navidrome/navidrome/adapters/lastfm" _ "github.com/navidrome/navidrome/adapters/listenbrainz" - _ "github.com/navidrome/navidrome/adapters/spotify" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" diff --git a/core/metrics/insights.go b/core/metrics/insights.go index f059d739a..5f3f491ea 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -199,7 +199,6 @@ var staticData = sync.OnceValue(func() insights.Data { data.Config.EnableSharing = conf.Server.EnableSharing data.Config.EnableStarRating = conf.Server.EnableStarRating data.Config.EnableLastFM = conf.Server.LastFM.Enabled && conf.Server.LastFM.ApiKey != "" && conf.Server.LastFM.Secret != "" - data.Config.EnableSpotify = conf.Server.Spotify.ID != "" && conf.Server.Spotify.Secret != "" data.Config.EnableListenBrainz = conf.Server.ListenBrainz.Enabled data.Config.EnableDeezer = conf.Server.Deezer.Enabled data.Config.EnableMediaFileCoverArt = conf.Server.EnableMediaFileCoverArt diff --git a/core/metrics/insights/data.go b/core/metrics/insights/data.go index 5580d895d..27186e020 100644 --- a/core/metrics/insights/data.go +++ b/core/metrics/insights/data.go @@ -61,7 +61,6 @@ type Data struct { EnableListenBrainz bool `json:"enableListenBrainz,omitempty"` EnableDeezer bool `json:"enableDeezer,omitempty"` EnableMediaFileCoverArt bool `json:"enableMediaFileCoverArt,omitempty"` - EnableSpotify bool `json:"enableSpotify,omitempty"` EnableJukebox bool `json:"enableJukebox,omitempty"` EnablePrometheus bool `json:"enablePrometheus,omitempty"` EnableCoverAnimation bool `json:"enableCoverAnimation,omitempty"` diff --git a/log/log.go b/log/log.go index 7d294792b..ef38484c8 100644 --- a/log/log.go +++ b/log/log.go @@ -27,7 +27,6 @@ var redacted = &Hook{ // Keys from the config "(ApiKey:\")[\\w]*", "(Secret:\")[\\w]*", - "(Spotify.*ID:\")[\\w]*", "(PasswordEncryptionKey:[\\s]*\")[^\"]*", "(UserHeader:[\\s]*\")[^\"]*", "(TrustedSources:[\\s]*\")[^\"]*", diff --git a/plugins/examples/wikimedia/README.md b/plugins/examples/wikimedia/README.md index e4833cff6..181055c69 100644 --- a/plugins/examples/wikimedia/README.md +++ b/plugins/examples/wikimedia/README.md @@ -63,7 +63,7 @@ Folder = "/path/to/navidrome/plugins" Add the plugin to your agents list: ```toml -Agents = "lastfm,spotify,wikimedia" +Agents = "lastfm,wikimedia" ``` ## Testing with Extism CLI diff --git a/plugins/manifest-schema.json b/plugins/manifest-schema.json index 8daf88ccf..c15a3bf3d 100644 --- a/plugins/manifest-schema.json +++ b/plugins/manifest-schema.json @@ -149,7 +149,7 @@ }, "requiredHosts": { "type": "array", - "description": "List of required host patterns for HTTP requests (e.g., 'api.example.com', '*.spotify.com')", + "description": "List of required host patterns for HTTP requests (e.g., 'api.example.com', '*.musicbrainz.org')", "items": { "type": "string" } @@ -189,7 +189,7 @@ }, "requiredHosts": { "type": "array", - "description": "List of required host patterns for WebSocket connections (e.g., 'api.example.com', '*.spotify.com')", + "description": "List of required host patterns for WebSocket connections (e.g., 'api.example.com', '*.musicbrainz.org')", "items": { "type": "string" } diff --git a/plugins/manifest_gen.go b/plugins/manifest_gen.go index a565ed3d1..efe93e05f 100644 --- a/plugins/manifest_gen.go +++ b/plugins/manifest_gen.go @@ -57,7 +57,7 @@ type HTTPPermission struct { Reason *string `json:"reason,omitempty" yaml:"reason,omitempty" mapstructure:"reason,omitempty"` // List of required host patterns for HTTP requests (e.g., 'api.example.com', - // '*.spotify.com') + // '*.musicbrainz.org') RequiredHosts []string `json:"requiredHosts,omitempty" yaml:"requiredHosts,omitempty" mapstructure:"requiredHosts,omitempty"` } @@ -251,6 +251,6 @@ type WebSocketPermission struct { Reason *string `json:"reason,omitempty" yaml:"reason,omitempty" mapstructure:"reason,omitempty"` // List of required host patterns for WebSocket connections (e.g., - // 'api.example.com', '*.spotify.com') + // 'api.example.com', '*.musicbrainz.org') RequiredHosts []string `json:"requiredHosts,omitempty" yaml:"requiredHosts,omitempty" mapstructure:"requiredHosts,omitempty"` } diff --git a/plugins/manifest_test.go b/plugins/manifest_test.go index de15324ab..c45a480eb 100644 --- a/plugins/manifest_test.go +++ b/plugins/manifest_test.go @@ -19,7 +19,7 @@ var _ = Describe("Manifest", func() { "permissions": { "http": { "reason": "Fetch metadata", - "requiredHosts": ["api.example.com", "*.spotify.com"] + "requiredHosts": ["api.example.com", "*.musicbrainz.org"] } } }`) @@ -34,7 +34,7 @@ var _ = Describe("Manifest", func() { Expect(*m.Website).To(Equal("https://example.com")) Expect(m.Permissions.Http).ToNot(BeNil()) Expect(*m.Permissions.Http.Reason).To(Equal("Fetch metadata")) - Expect(m.Permissions.Http.RequiredHosts).To(ContainElements("api.example.com", "*.spotify.com")) + Expect(m.Permissions.Http.RequiredHosts).To(ContainElements("api.example.com", "*.musicbrainz.org")) }) It("parses a minimal manifest", func() { diff --git a/server/initial_setup.go b/server/initial_setup.go index ebfdad47a..d50f25958 100644 --- a/server/initial_setup.go +++ b/server/initial_setup.go @@ -91,11 +91,5 @@ func checkExternalCredentials() { } else { log.Debug("ListenBrainz integration is ENABLED", "ListenBrainz.BaseURL", conf.Server.ListenBrainz.BaseURL) } - - if conf.Server.Spotify.ID == "" || conf.Server.Spotify.Secret == "" { - log.Info("Spotify integration is not enabled: missing ID/Secret") - } else { - log.Debug("Spotify integration is ENABLED") - } } } diff --git a/server/nativeapi/config.go b/server/nativeapi/config.go index 086e8d3c1..02626a4ee 100644 --- a/server/nativeapi/config.go +++ b/server/nativeapi/config.go @@ -16,13 +16,11 @@ import ( // using partial masking (first and last character visible, middle replaced with *). // For values with 7+ characters: "secretvalue123" becomes "s***********3" // For values with <7 characters: "short" becomes "****" -// Add field paths using dot notation (e.g., "LastFM.ApiKey", "Spotify.Secret") +// Add field paths using dot notation (e.g., "LastFM.ApiKey") var sensitiveFieldsPartialMask = []string{ "LastFM.ApiKey", "LastFM.Secret", "Prometheus.MetricsPath", - "Spotify.ID", - "Spotify.Secret", "DevAutoLoginUsername", } diff --git a/server/nativeapi/config_test.go b/server/nativeapi/config_test.go index 546dd4f12..d7368cabf 100644 --- a/server/nativeapi/config_test.go +++ b/server/nativeapi/config_test.go @@ -78,7 +78,6 @@ var _ = Describe("Config API", func() { It("redacts sensitive fields", func() { conf.Server.LastFM.ApiKey = "secretapikey123" - conf.Server.Spotify.Secret = "spotifysecret456" conf.Server.PasswordEncryptionKey = "encryptionkey789" conf.Server.DevAutoCreateAdminPassword = "adminpassword123" conf.Server.Prometheus.Password = "prometheuspass" @@ -97,11 +96,6 @@ var _ = Describe("Config API", func() { Expect(ok).To(BeTrue()) Expect(lastfm["ApiKey"]).To(Equal("s*************3")) - // Check Spotify.Secret (partially masked) - spotify, ok := resp.Config["Spotify"].(map[string]any) - Expect(ok).To(BeTrue()) - Expect(spotify["Secret"]).To(Equal("s**************6")) - // Check PasswordEncryptionKey (fully masked) Expect(resp.Config["PasswordEncryptionKey"]).To(Equal("****")) @@ -172,7 +166,6 @@ var _ = Describe("Config API", func() { var _ = Describe("redactValue function", func() { It("partially masks long sensitive values", func() { Expect(redactValue("LastFM.ApiKey", "ba46f0e84a")).To(Equal("b********a")) - Expect(redactValue("Spotify.Secret", "verylongsecret123")).To(Equal("v***************3")) }) It("fully masks long sensitive values that should be completely hidden", func() { @@ -183,7 +176,6 @@ var _ = Describe("redactValue function", func() { It("fully masks short sensitive values", func() { Expect(redactValue("LastFM.Secret", "short")).To(Equal("****")) - Expect(redactValue("Spotify.ID", "abc")).To(Equal("****")) Expect(redactValue("PasswordEncryptionKey", "12345")).To(Equal("****")) Expect(redactValue("DevAutoCreateAdminPassword", "short")).To(Equal("****")) Expect(redactValue("Prometheus.Password", "short")).To(Equal("****")) From a887521d7a27ec626c04c557e9591a9f2e3819d1 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 15 Mar 2026 13:36:26 -0400 Subject: [PATCH 2/5] fix(subsonic): always include mandatory title field in Child responses Removed `omitempty` from the `Title` struct tag in the `Child` response type. The Subsonic/OpenSubsonic API spec requires `title` to be a mandatory field, but songs with empty titles caused the field to be omitted entirely, crashing clients like Symfonium during sync. Ref: https://support.symfonium.app/t/app-gets-stuck-on-syncing-large-database/13004/8 --- server/subsonic/helpers_test.go | 8 ++++++++ .../Responses AlbumList with OS data should match .JSON | 1 + .../Responses AlbumList with OS data should match .XML | 2 +- .../Responses Child with data should match .JSON | 1 + .../Responses Child with data should match .XML | 2 +- .../Responses Child without data should match .JSON | 3 ++- .../Responses Child without data should match .XML | 2 +- ...ses Child without data should match OpenSubsonic .JSON | 1 + ...nses Child without data should match OpenSubsonic .XML | 2 +- server/subsonic/responses/responses.go | 2 +- 10 files changed, 18 insertions(+), 6 deletions(-) diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go index d20ca89ad..4eb756b98 100644 --- a/server/subsonic/helpers_test.go +++ b/server/subsonic/helpers_test.go @@ -309,6 +309,14 @@ var _ = Describe("helpers", func() { Expect(child.Artist).To(Equal("Test Artist")) }) }) + + Context("when MediaFile has an empty title", func() { + It("still includes the title field in the response", func() { + mf.Title = "" + child := childFromMediaFile(ctx, mf) + Expect(child.Title).To(Equal("")) + }) + }) }) Describe("osChildFromMediaFile", func() { diff --git a/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .JSON b/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .JSON index a50603bf9..8491a577b 100644 --- a/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .JSON +++ b/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .JSON @@ -9,6 +9,7 @@ { "id": "1", "isDir": false, + "title": "", "bpm": 0, "comment": "", "sortName": "sort name", diff --git a/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .XML b/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .XML index 45b7033f3..5d9e83f96 100644 --- a/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .XML +++ b/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .XML @@ -1,6 +1,6 @@ - + mood1 diff --git a/server/subsonic/responses/.snapshots/Responses Child with data should match .JSON b/server/subsonic/responses/.snapshots/Responses Child with data should match .JSON index 448a84d6a..d20a6d48c 100644 --- a/server/subsonic/responses/.snapshots/Responses Child with data should match .JSON +++ b/server/subsonic/responses/.snapshots/Responses Child with data should match .JSON @@ -115,6 +115,7 @@ { "id": "", "isDir": false, + "title": "", "bpm": 0, "comment": "", "sortName": "", diff --git a/server/subsonic/responses/.snapshots/Responses Child with data should match .XML b/server/subsonic/responses/.snapshots/Responses Child with data should match .XML index 2de1efbfb..1d307b0b9 100644 --- a/server/subsonic/responses/.snapshots/Responses Child with data should match .XML +++ b/server/subsonic/responses/.snapshots/Responses Child with data should match .XML @@ -25,7 +25,7 @@ - + diff --git a/server/subsonic/responses/.snapshots/Responses Child without data should match .JSON b/server/subsonic/responses/.snapshots/Responses Child without data should match .JSON index 2d0995831..b66a2bdea 100644 --- a/server/subsonic/responses/.snapshots/Responses Child without data should match .JSON +++ b/server/subsonic/responses/.snapshots/Responses Child without data should match .JSON @@ -8,7 +8,8 @@ "child": [ { "id": "1", - "isDir": false + "isDir": false, + "title": "" } ], "id": "", diff --git a/server/subsonic/responses/.snapshots/Responses Child without data should match .XML b/server/subsonic/responses/.snapshots/Responses Child without data should match .XML index 3e5a1cf13..d64d526d6 100644 --- a/server/subsonic/responses/.snapshots/Responses Child without data should match .XML +++ b/server/subsonic/responses/.snapshots/Responses Child without data should match .XML @@ -1,5 +1,5 @@ - + diff --git a/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .JSON b/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .JSON index c3ccc6cd0..25284295e 100644 --- a/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .JSON +++ b/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .JSON @@ -9,6 +9,7 @@ { "id": "1", "isDir": false, + "title": "", "bpm": 0, "comment": "", "sortName": "", diff --git a/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .XML b/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .XML index 3e5a1cf13..d64d526d6 100644 --- a/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .XML +++ b/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .XML @@ -1,5 +1,5 @@ - + diff --git a/server/subsonic/responses/responses.go b/server/subsonic/responses/responses.go index c550ce61e..b70f3b128 100644 --- a/server/subsonic/responses/responses.go +++ b/server/subsonic/responses/responses.go @@ -135,7 +135,7 @@ type Child struct { Id string `xml:"id,attr" json:"id"` Parent string `xml:"parent,attr,omitempty" json:"parent,omitempty"` IsDir bool `xml:"isDir,attr" json:"isDir"` - Title string `xml:"title,attr,omitempty" json:"title,omitempty"` + Title string `xml:"title,attr" json:"title"` Name string `xml:"name,attr,omitempty" json:"name,omitempty"` Album string `xml:"album,attr,omitempty" json:"album,omitempty"` Artist string `xml:"artist,attr,omitempty" json:"artist,omitempty"` From c42570446b80d019840c59ba962da28c781cc76d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 15 Mar 2026 14:00:21 -0400 Subject: [PATCH 3/5] fix(ui): allow DefaultTheme "Auto" from config (#5190) * fix(ui): allow DefaultTheme "Auto" from config When DefaultTheme is set to "Auto" in the server config, the defaultTheme() function in themeReducer now returns AUTO_THEME_ID instead of falling through to the DarkTheme fallback. This allows useCurrentTheme to correctly read prefers-color-scheme and select Light or Dark theme automatically for new/incognito users. Adds themeReducer unit tests covering Auto, named-theme, and unrecognized-value fallback paths. * chore: format Signed-off-by: Deluan --------- Signed-off-by: Deluan Co-authored-by: Deluan --- ui/src/consts.js | 2 ++ ui/src/reducers/themeReducer.js | 4 ++++ ui/src/reducers/themeReducer.test.js | 32 ++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+) create mode 100644 ui/src/reducers/themeReducer.test.js diff --git a/ui/src/consts.js b/ui/src/consts.js index e3446c2fe..30731a080 100644 --- a/ui/src/consts.js +++ b/ui/src/consts.js @@ -7,6 +7,8 @@ export const M3U_MIME_TYPE = 'audio/x-mpegurl' export const AUTO_THEME_ID = 'AUTO_THEME_ID' +export const AUTO_THEME_CONFIG_VALUE = 'Auto' + export const DraggableTypes = { SONG: 'song', ALBUM: 'album', diff --git a/ui/src/reducers/themeReducer.js b/ui/src/reducers/themeReducer.js index 2a5d5bac6..16d5fa87b 100644 --- a/ui/src/reducers/themeReducer.js +++ b/ui/src/reducers/themeReducer.js @@ -1,8 +1,12 @@ import { CHANGE_THEME } from '../actions' +import { AUTO_THEME_ID, AUTO_THEME_CONFIG_VALUE } from '../consts' import config from '../config' import themes from '../themes' const defaultTheme = () => { + if (config.defaultTheme === AUTO_THEME_CONFIG_VALUE) { + return AUTO_THEME_ID + } return ( Object.keys(themes).find( (t) => themes[t].themeName === config.defaultTheme, diff --git a/ui/src/reducers/themeReducer.test.js b/ui/src/reducers/themeReducer.test.js new file mode 100644 index 000000000..2a66ea851 --- /dev/null +++ b/ui/src/reducers/themeReducer.test.js @@ -0,0 +1,32 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { AUTO_THEME_ID, AUTO_THEME_CONFIG_VALUE } from '../consts' + +describe('themeReducer', () => { + beforeEach(() => { + vi.resetModules() + }) + + it.each([ + { + configTheme: AUTO_THEME_CONFIG_VALUE, + expected: AUTO_THEME_ID, + description: 'is "Auto"', + }, + { configTheme: 'Dark', expected: 'DarkTheme', description: 'is "Dark"' }, + { + configTheme: 'NonExistent', + expected: 'DarkTheme', + description: 'is unrecognized', + }, + ])( + 'returns $expected when defaultTheme config $description', + async ({ configTheme, expected }) => { + vi.doMock('../config', () => ({ + default: { defaultTheme: configTheme }, + })) + const { themeReducer } = await import('./themeReducer') + const result = themeReducer(undefined, { type: 'UNKNOWN' }) + expect(result).toBe(expected) + }, + ) +}) From aa9391199182bb28fa4ffa222ec2a5fa02b77ee9 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 15 Mar 2026 14:14:05 -0400 Subject: [PATCH 4/5] feat(server): add syslog priority prefixes for systemd-journald (#5192) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: add syslog priority prefixes for systemd-journald When running under systemd, all log messages were assigned priority 3 (error) by journald because navidrome wrote plain text to stderr without syslog priority prefixes. Add a journalFormatter that wraps the existing logrus formatter and prepends syslog priority prefixes (RFC 5424) to each log line. The formatter is automatically enabled when the JOURNAL_STREAM environment variable is set (indicating the process is managed by systemd). Priority mapping: - Fatal/Panic → <2>/<0> (crit/emerg) - Error → <3> (err) - Warn → <4> (warning) - Info → <6> (info) - Debug/Trace → <7> (debug) Fixes #5142 * test: refactor journalFormatter tests to use Ginkgo and DescribeTable Signed-off-by: Deluan --------- Signed-off-by: Deluan Co-authored-by: Deluan --- conf/configuration.go | 4 ++++ log/journal.go | 41 +++++++++++++++++++++++++++++++++++++++++ log/journal_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ log/log.go | 9 +++++++++ 4 files changed, 95 insertions(+) create mode 100644 log/journal.go create mode 100644 log/journal_test.go diff --git a/conf/configuration.go b/conf/configuration.go index 986ab7a5a..dacd42539 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -340,6 +340,10 @@ func Load(noConfigDump bool) { os.Exit(1) } log.SetOutput(out) + } else if os.Getenv("JOURNAL_STREAM") != "" { + // When running under systemd, prepend syslog priority prefixes so + // journald assigns the correct severity to each log line. + log.EnableJournalFormat() } log.SetLevelString(Server.LogLevel) diff --git a/log/journal.go b/log/journal.go new file mode 100644 index 000000000..f1c17d2e7 --- /dev/null +++ b/log/journal.go @@ -0,0 +1,41 @@ +package log + +import ( + "fmt" + + "github.com/sirupsen/logrus" +) + +// journalFormatter wraps a logrus.Formatter and prepends a syslog priority +// prefix () to each log line. When stderr is captured by systemd-journald, +// this prefix tells journald the correct severity for each message. +// +// See https://www.freedesktop.org/software/systemd/man/sd-daemon.html +type journalFormatter struct { + inner logrus.Formatter +} + +// levelToPriority maps logrus levels to syslog priority values. +// The mapping follows RFC 5424 severity levels. +var levelToPriority = map[logrus.Level]int{ + logrus.PanicLevel: 0, // emerg + logrus.FatalLevel: 2, // crit + logrus.ErrorLevel: 3, // err + logrus.WarnLevel: 4, // warning + logrus.InfoLevel: 6, // info + logrus.DebugLevel: 7, // debug + logrus.TraceLevel: 7, // debug +} + +func (f *journalFormatter) Format(entry *logrus.Entry) ([]byte, error) { + formatted, err := f.inner.Format(entry) + if err != nil { + return formatted, err + } + priority, ok := levelToPriority[entry.Level] + if !ok { + priority = 6 // default to info for unknown levels + } + prefix := []byte(fmt.Sprintf("<%d>", priority)) + return append(prefix, formatted...), nil +} diff --git a/log/journal_test.go b/log/journal_test.go new file mode 100644 index 000000000..770f12b6d --- /dev/null +++ b/log/journal_test.go @@ -0,0 +1,41 @@ +package log + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/sirupsen/logrus" +) + +var _ = Describe("journalFormatter", func() { + var formatter *journalFormatter + + BeforeEach(func() { + inner := &logrus.TextFormatter{ + DisableTimestamp: true, + DisableColors: true, + } + formatter = &journalFormatter{inner: inner} + }) + + DescribeTable("prefixes log lines with syslog priority", + func(level logrus.Level, expectedPrefix string) { + entry := &logrus.Entry{ + Logger: logrus.New(), + Level: level, + Message: "test message", + Data: logrus.Fields{}, + } + out, err := formatter.Format(entry) + Expect(err).ToNot(HaveOccurred()) + Expect(string(out)).To(HavePrefix(expectedPrefix)) + }, + Entry("error", logrus.ErrorLevel, "<3>"), + Entry("warning", logrus.WarnLevel, "<4>"), + Entry("info", logrus.InfoLevel, "<6>"), + Entry("debug", logrus.DebugLevel, "<7>"), + Entry("trace", logrus.TraceLevel, "<7>"), + Entry("fatal", logrus.FatalLevel, "<2>"), + Entry("panic", logrus.PanicLevel, "<0>"), + Entry("unknown level defaults to info", logrus.Level(99), "<6>"), + ) +}) diff --git a/log/log.go b/log/log.go index ef38484c8..2764d80e5 100644 --- a/log/log.go +++ b/log/log.go @@ -145,6 +145,15 @@ func SetOutput(w io.Writer) { defaultLogger.SetOutput(w) } +// EnableJournalFormat wraps the current logger formatter with syslog +// priority prefixes for systemd-journald. Only call this when output +// goes to stderr and JOURNAL_STREAM is set. +func EnableJournalFormat() { + loggerMu.Lock() + defer loggerMu.Unlock() + defaultLogger.Formatter = &journalFormatter{inner: defaultLogger.Formatter} +} + // Redact applies redaction to a single string func Redact(msg string) string { r, _ := redacted.redact(msg) From 36aea8a11fbccc2efca56f5ecd88545059ec90ac Mon Sep 17 00:00:00 2001 From: Thiago Sfredo Date: Sun, 15 Mar 2026 15:55:55 -0300 Subject: [PATCH 5/5] feat(ui): add tooltips for long playlist and album names - 5068 (#5070) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * style(ui): add tooltips for long playlist and album names - 5068 Signed-off-by: Thiago Sfreddo * fix dnd and improve performance Signed-off-by: Thiago Sfreddo * lint fixes Signed-off-by: Thiago Sfreddo * fix(ui): update tooltip styles for improved visibility and consistency Signed-off-by: Deluan * fix(ui): add overflow tooltip to playlist name for better visibility Signed-off-by: Deluan * refactor(ui): simplify OverflowTooltip and improve render performance - Inline styles from useMenuTooltipStyles into OverflowTooltip (single consumer) - Use MUI named colors (grey[700]/grey[300] with alpha) instead of raw rgba - Stabilize ref callback with useCallback to avoid unnecessary ref churn - Memoize Tooltip classes and hoist TransitionProps to module level - Fix useLayoutEffect dependency: observe DOM size, not title string --------- Signed-off-by: Thiago Sfreddo Signed-off-by: Deluan Co-authored-by: Deluan Quintão --- ui/src/album/AlbumGridView.jsx | 11 +++- ui/src/common/OverflowTooltip.jsx | 90 +++++++++++++++++++++++++++++ ui/src/common/index.js | 1 + ui/src/layout/PlaylistsSubMenu.jsx | 10 ++-- ui/src/playlist/PlaylistDetails.jsx | 15 +++-- 5 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 ui/src/common/OverflowTooltip.jsx diff --git a/ui/src/album/AlbumGridView.jsx b/ui/src/album/AlbumGridView.jsx index 58732bbde..b7db39730 100644 --- a/ui/src/album/AlbumGridView.jsx +++ b/ui/src/album/AlbumGridView.jsx @@ -13,7 +13,12 @@ import { linkToRecord, useListContext, Loading } from 'react-admin' import { withContentRect } from 'react-measure' import { useDrag } from 'react-dnd' import subsonic from '../subsonic' -import { AlbumContextMenu, PlayButton, ArtistLinkField } from '../common' +import { + AlbumContextMenu, + PlayButton, + ArtistLinkField, + OverflowTooltip, +} from '../common' import { DraggableTypes } from '../consts' import clsx from 'clsx' import { AlbumDatesField } from './AlbumDatesField.jsx' @@ -198,7 +203,9 @@ const AlbumGridTile = ({ showArtist, record, basePath, ...props }) => { to={linkToRecord(basePath, record.id, 'show')} > - {record.name} + + {record.name} + {record.tags && record.tags['albumversion'] && ( {record.tags['albumversion']} diff --git a/ui/src/common/OverflowTooltip.jsx b/ui/src/common/OverflowTooltip.jsx new file mode 100644 index 000000000..c000bd9f0 --- /dev/null +++ b/ui/src/common/OverflowTooltip.jsx @@ -0,0 +1,90 @@ +import React from 'react' +import PropTypes from 'prop-types' +import { Tooltip } from '@material-ui/core' +import { makeStyles, alpha } from '@material-ui/core/styles' +import grey from '@material-ui/core/colors/grey' + +const useStyles = makeStyles( + (theme) => ({ + tooltip: { + backgroundColor: + theme.palette.type === 'dark' + ? alpha(grey[700], 0.92) + : alpha(grey[300], 0.92), + color: + theme.palette.type === 'dark' + ? theme.palette.common.white + : theme.palette.common.black, + borderRadius: theme.shape.borderRadius, + ...theme.typography.body2, + padding: theme.spacing(0.5, 1), + maxWidth: 300, + }, + }), + { name: 'NDOverflowTooltip' }, +) + +const transitionProps = { timeout: 0 } + +export const OverflowTooltip = ({ + children, + title, + placement = 'bottom-start', +}) => { + const classes = useStyles() + const textRef = React.useRef(null) + const [isOverflowing, setIsOverflowing] = React.useState(false) + const tooltipClasses = React.useMemo( + () => ({ tooltip: classes.tooltip }), + [classes.tooltip], + ) + + React.useLayoutEffect(() => { + const el = textRef.current + if (!el) return + + const checkOverflow = () => { + setIsOverflowing(el.scrollWidth > el.clientWidth) + } + + const resizeObserver = new ResizeObserver(checkOverflow) + resizeObserver.observe(el) + + checkOverflow() + + return () => resizeObserver.disconnect() + }, []) + + const mergedRef = React.useCallback( + (el) => { + textRef.current = el + + const { ref } = children + if (typeof ref === 'function') { + ref(el) + } else if (ref && typeof ref === 'object') { + ref.current = el + } + }, + [children], + ) + + return ( + + {React.cloneElement(children, { ref: mergedRef })} + + ) +} + +OverflowTooltip.propTypes = { + children: PropTypes.element.isRequired, + title: PropTypes.string.isRequired, + placement: PropTypes.string, +} diff --git a/ui/src/common/index.js b/ui/src/common/index.js index 356225680..a8dc354cb 100644 --- a/ui/src/common/index.js +++ b/ui/src/common/index.js @@ -41,4 +41,5 @@ export * from './formatRange.js' export * from './playlistUtils.js' export * from './PathField.jsx' export * from './ParticipantsInfo' +export * from './OverflowTooltip' export * from './useSearchRefocus' diff --git a/ui/src/layout/PlaylistsSubMenu.jsx b/ui/src/layout/PlaylistsSubMenu.jsx index a9f70b875..b94bebf86 100644 --- a/ui/src/layout/PlaylistsSubMenu.jsx +++ b/ui/src/layout/PlaylistsSubMenu.jsx @@ -12,7 +12,7 @@ import QueueMusicOutlinedIcon from '@material-ui/icons/QueueMusicOutlined' import { BiCog } from 'react-icons/bi' import { useDrop } from 'react-dnd' import SubMenu from './SubMenu' -import { canChangeTracks } from '../common' +import { canChangeTracks, OverflowTooltip } from '../common' import { DraggableTypes } from '../consts' import config from '../config' @@ -39,9 +39,11 @@ const PlaylistMenuItemLink = ({ pls, sidebarIsOpen }) => { - {pls.name} - + + + {pls.name} + + } sidebarIsOpen={sidebarIsOpen} dense={false} diff --git a/ui/src/playlist/PlaylistDetails.jsx b/ui/src/playlist/PlaylistDetails.jsx index eefed83b4..aeddf0d42 100644 --- a/ui/src/playlist/PlaylistDetails.jsx +++ b/ui/src/playlist/PlaylistDetails.jsx @@ -19,6 +19,7 @@ import { DurationField, SizeField, isWritable, + OverflowTooltip, } from '../common' import config from '../config' import subsonic from '../subsonic' @@ -274,12 +275,14 @@ const PlaylistDetails = (props) => {
- - {record.name || translate('ra.page.loading')} - + + + {record.name || translate('ra.page.loading')} + + {record.songCount ? (