diff --git a/cmd/root.go b/cmd/root.go index c4e360010..f632b0425 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -86,6 +86,7 @@ func runNavidrome(ctx context.Context) { g.Go(startSignaller(ctx)) g.Go(startScheduler(ctx)) g.Go(startPlaybackServer(ctx)) + g.Go(startJellyfinDiscovery(ctx)) g.Go(schedulePeriodicBackup(ctx)) g.Go(startInsightsCollector(ctx)) g.Go(scheduleDBAnalyzer(ctx)) @@ -343,6 +344,18 @@ func startInsightsCollector(ctx context.Context) func() error { } } +// startJellyfinDiscovery never returns an error: a discovery failure must not stop the server. +func startJellyfinDiscovery(ctx context.Context) func() error { + return func() error { + if !conf.Server.Jellyfin.Enabled || !conf.Server.Jellyfin.AutoDiscovery { + log.Debug("Jellyfin auto-discovery is DISABLED") + return nil + } + CreateJellyfinDiscovery().Serve(ctx) + return nil + } +} + // startPlaybackServer starts the Navidrome playback server, if configured. // It is responsible for the Jukebox functionality func startPlaybackServer(ctx context.Context) func() error { diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index fd04c44c5..d6bf4da07 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -168,6 +168,13 @@ func CreateListenBrainzRouter() *listenbrainz.Router { return router } +func CreateJellyfinDiscovery() *jellyfin.Discovery { + sqlDB := db.Db() + dataStore := persistence.New(sqlDB) + discovery := jellyfin.NewDiscovery(dataStore) + return discovery +} + func CreateInsights() metrics.Insights { sqlDB := db.Db() dataStore := persistence.New(sqlDB) @@ -249,7 +256,7 @@ func getPluginManager() *plugins.Manager { // wire_injectors.go: -var allProviders = wire.NewSet(core.Set, artwork.Set, server.New, subsonic.New, jellyfin.New, nativeapi.New, public.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, scanner.GetInstance, scanner.GetWatcher, metrics.GetPrometheusInstance, db.Db, plugins.GetManager, sonic.New, wire.Bind(new(agents.PluginLoader), new(*plugins.Manager)), wire.Bind(new(scrobbler.PluginLoader), new(*plugins.Manager)), wire.Bind(new(lyrics.PluginLoader), new(*plugins.Manager)), wire.Bind(new(sonic.PluginLoader), new(*plugins.Manager)), wire.Bind(new(sonic.Engine), new(*sonic.Sonic)), wire.Bind(new(nativeapi.PluginManager), new(*plugins.Manager)), wire.Bind(new(core.PluginUnloader), new(*plugins.Manager)), wire.Bind(new(plugins.PluginMetricsRecorder), new(metrics.Metrics)), wire.Bind(new(core.Watcher), new(scanner.Watcher)), wire.Bind(new(playlists.ImageUploadService), new(artwork.Uploader))) +var allProviders = wire.NewSet(core.Set, artwork.Set, server.New, subsonic.New, jellyfin.New, jellyfin.NewDiscovery, nativeapi.New, public.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, scanner.GetInstance, scanner.GetWatcher, metrics.GetPrometheusInstance, db.Db, plugins.GetManager, sonic.New, wire.Bind(new(agents.PluginLoader), new(*plugins.Manager)), wire.Bind(new(scrobbler.PluginLoader), new(*plugins.Manager)), wire.Bind(new(lyrics.PluginLoader), new(*plugins.Manager)), wire.Bind(new(sonic.PluginLoader), new(*plugins.Manager)), wire.Bind(new(sonic.Engine), new(*sonic.Sonic)), wire.Bind(new(nativeapi.PluginManager), new(*plugins.Manager)), wire.Bind(new(core.PluginUnloader), new(*plugins.Manager)), wire.Bind(new(plugins.PluginMetricsRecorder), new(metrics.Metrics)), wire.Bind(new(core.Watcher), new(scanner.Watcher)), wire.Bind(new(playlists.ImageUploadService), new(artwork.Uploader))) func GetPluginManager(ctx context.Context) *plugins.Manager { manager := getPluginManager() diff --git a/cmd/wire_injectors.go b/cmd/wire_injectors.go index a54d4ae1b..527617959 100644 --- a/cmd/wire_injectors.go +++ b/cmd/wire_injectors.go @@ -36,6 +36,7 @@ var allProviders = wire.NewSet( server.New, subsonic.New, jellyfin.New, + jellyfin.NewDiscovery, nativeapi.New, public.New, persistence.New, @@ -108,6 +109,12 @@ func CreateListenBrainzRouter() *listenbrainz.Router { )) } +func CreateJellyfinDiscovery() *jellyfin.Discovery { + panic(wire.Build( + allProviders, + )) +} + func CreateInsights() metrics.Insights { panic(wire.Build( allProviders, diff --git a/conf/configuration.go b/conf/configuration.go index e549e2a61..4812f2515 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -235,6 +235,7 @@ type jellyfinOptions struct { // ExposedPublicUsers is a comma-separated list of usernames to advertise on the unauthenticated // GET /Users/Public, so Jellyfin clients can show a login user-picker. Empty exposes no users. ExposedPublicUsers string + AutoDiscovery bool // MaxConcurrentStreams bounds how many collection responses can stream at once. Each holds a DB // cursor — and its pooled connection — for the whole client-paced response, so without a bound // enough slow clients would take the entire pool and stall the scanner, scrobbles and the UI. @@ -1087,6 +1088,7 @@ func setViperDefaults() { viper.SetDefault("listenbrainz.trackalgorithm", consts.DefaultListenBrainzTrackAlgorithm) viper.SetDefault("jellyfin.enabled", false) viper.SetDefault("jellyfin.servername", "") + viper.SetDefault("jellyfin.autodiscovery", false) viper.SetDefault("enablescrobblehistory", true) viper.SetDefault("httpheaders.frameoptions", "DENY") viper.SetDefault("backup.path", "") diff --git a/server/jellyfin/README.md b/server/jellyfin/README.md index a20c9ce4e..893642cdd 100644 --- a/server/jellyfin/README.md +++ b/server/jellyfin/README.md @@ -22,6 +22,8 @@ Enabled = true ServerName = "My Music Server" # Optional: usernames to show in the client login user-picker (default: none). See "Public user list". ExposedPublicUsers = "alice, bob" +# Optional: answer LAN auto-discovery broadcasts on UDP 7359 (default: false). See "Auto discovery". +AutoDiscovery = true # Optional: max collection responses streaming at once (default: half the DB connection pool, # min 2). Each streaming response holds a DB connection for its whole duration; excess requests # queue rather than fail. @@ -34,6 +36,7 @@ or via environment variables: ND_JELLYFIN_ENABLED=true ND_JELLYFIN_SERVERNAME="My Music Server" ND_JELLYFIN_EXPOSEDPUBLICUSERS="alice,bob" +ND_JELLYFIN_AUTODISCOVERY=true ``` Once enabled, the API is mounted at: @@ -46,6 +49,25 @@ All the paths below are relative to that base URL (e.g. `System/Info/Public` mea `http://localhost:4533/jellyfin/System/Info/Public`). Routes are matched **case-insensitively**, since real Jellyfin clients (and `jellyfin-apiclient-python`) send mixed-case paths. +## Auto discovery + +With `AutoDiscovery = true`, Navidrome answers the Jellyfin LAN discovery broadcast +(`who is JellyfinServer?` on UDP port 7359), so clients list the server without a typed URL. +It is off by default because a real Jellyfin server on the same host owns that port. If the port +is taken, Navidrome logs a warning and keeps running without discovery. + +The advertised address is `BaseURL` when it includes a host. Otherwise it is the bind `Address` +when that is a specific IP, or else the local IP that faces the requesting client, plus `Port`. With a +unix socket `Address` there is no port to advertise, so discovery only starts when `BaseURL` has a host. + +Discovery answers on all IPv4 interfaces, but the advertised address follows `BaseURL`, `Address` and +`Port`. If `Address` is a loopback or a single interface IP and `BaseURL` has no host, clients on other +networks get an address they cannot reach. Set `BaseURL` to the address clients should use. + +Docker: publish the port (`-p 7359:7359/udp`). In bridge mode the server only sees its container +IP, so also set `ND_BASEURL` to the LAN address (for example `http://192.168.1.10:4533`), or use +host networking. Keep UDP 7359 on the LAN: never forward it from the internet. + ## Authentication Jellyfin clients authenticate with `POST /Users/AuthenticateByName` using the user's Navidrome diff --git a/server/jellyfin/api.go b/server/jellyfin/api.go index 955ebc3bb..eddd14f66 100644 --- a/server/jellyfin/api.go +++ b/server/jellyfin/api.go @@ -3,7 +3,6 @@ package jellyfin import ( "encoding/json" "net/http" - "sync" "time" "github.com/go-chi/chi/v5" @@ -41,7 +40,6 @@ type Router struct { broker events.Broker lyricsCache cache.SimpleCache[string, model.LyricList] similarFlight singleflight.Group - serverIDMu sync.Mutex serverIDVal string } diff --git a/server/jellyfin/auth.go b/server/jellyfin/auth.go index bfcfab319..ba4fe40f1 100644 --- a/server/jellyfin/auth.go +++ b/server/jellyfin/auth.go @@ -45,7 +45,7 @@ func (api *Router) authenticateByName(w http.ResponseWriter, r *http.Request) { a := parseMediaBrowserAuth(r) serverID := api.serverID(ctx) api.ok(w, r, dto.AuthenticationResult{ - User: userToDto(usr, api.serverName(), serverID), + User: userToDto(usr, serverName(), serverID), SessionInfo: dto.NewSessionInfo(usr, a.Client, a.DeviceId, a.Device, a.Version, serverID), AccessToken: token, ServerId: serverID, diff --git a/server/jellyfin/discovery.go b/server/jellyfin/discovery.go new file mode 100644 index 000000000..d111287a6 --- /dev/null +++ b/server/jellyfin/discovery.go @@ -0,0 +1,110 @@ +package jellyfin + +import ( + "context" + "encoding/json" + "net" + "strconv" + "strings" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core/publicurl" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/utils/gg" +) + +// Jellyfin clients broadcast this query text to this UDP port. +const ( + discoveryPort = 7359 + discoveryQuery = "who is jellyfinserver?" +) + +type discoveryInfo struct { + Address string `json:"Address"` + Id string `json:"Id"` + Name string `json:"Name"` + EndpointAddress *string `json:"EndpointAddress"` +} + +// Discovery answers LAN auto-discovery broadcasts with the same identity the Router reports. +type Discovery struct { + ds model.DataStore + serverIDVal string +} + +func NewDiscovery(ds model.DataStore) *Discovery { + return &Discovery{ds: ds} +} + +func (d *Discovery) serverID(ctx context.Context) string { + return resolveServerID(ctx, d.ds, &d.serverIDVal) +} + +// Serve runs until ctx is done. A failed bind is only logged: discovery is best-effort. +func (d *Discovery) Serve(ctx context.Context) { + if !hasAdvertisableAddress() { + log.Warn(ctx, "Jellyfin API: auto-discovery is off, a unix socket server needs a BaseURL with a host to advertise") + return + } + // udp4 only: a dual-stack bind can share the port with another server and never get a packet. + conn, err := net.ListenPacket("udp4", net.JoinHostPort("0.0.0.0", strconv.Itoa(discoveryPort))) + if err != nil { + log.Warn(ctx, "Jellyfin API: auto-discovery is off, the UDP port is unavailable. Is another Jellyfin server running?", "port", discoveryPort, err) + return + } + log.Info(ctx, "Jellyfin API: listening for auto-discovery broadcasts", "port", discoveryPort) + d.ServeOn(ctx, conn) +} + +// ServeOn answers discovery queries on conn until ctx is done, then closes conn. +func (d *Discovery) ServeOn(ctx context.Context, conn net.PacketConn) { + defer conn.Close() + stop := context.AfterFunc(ctx, func() { _ = conn.Close() }) + defer stop() + buf := make([]byte, 1024) + for { + n, remote, err := conn.ReadFrom(buf) + if err != nil { + if ctx.Err() == nil { + log.Error(ctx, "Jellyfin API: auto-discovery listener stopped", err) + } + return + } + if !strings.Contains(strings.ToLower(string(buf[:n])), discoveryQuery) { + continue + } + info := discoveryInfo{Address: discoveryAddress(ctx, remote), Id: d.serverID(ctx), Name: serverName()} + res, _ := json.Marshal(info) + log.Debug(ctx, "Jellyfin API: answering auto-discovery request", "from", remote.String(), "address", info.Address) + if _, err := conn.WriteTo(res, remote); err != nil { + log.Debug(ctx, "Jellyfin API: could not answer auto-discovery request", "to", remote.String(), err) + } + } +} + +// Behind a unix socket nothing listens on Port, so only a BaseURL host gives clients an address. +func hasAdvertisableAddress() bool { + return conf.Server.BaseHost != "" || !strings.HasPrefix(conf.Server.Address, "unix:") +} + +func discoveryAddress(ctx context.Context, remote net.Addr) string { + scheme := gg.If(conf.Server.TLSEnabled(), "https", "http") + host := net.JoinHostPort(localIPFor(remote), strconv.Itoa(conf.Server.Port)) + return publicurl.AbsoluteURL(request.WithServerAddress(ctx, scheme, host), consts.URLPathJellyfinAPI, nil) +} + +// On a multi-homed host, only the interface that routes to the requester is reachable by it. +func localIPFor(remote net.Addr) string { + if ip := parseIP(conf.Server.Address); ip.IsValid() && !ip.IsUnspecified() { + return ip.String() + } + c, err := net.Dial("udp", remote.String()) + if err != nil { + return conf.Server.Address + } + defer c.Close() + return c.LocalAddr().(*net.UDPAddr).IP.String() +} diff --git a/server/jellyfin/discovery_test.go b/server/jellyfin/discovery_test.go new file mode 100644 index 000000000..bcd11fe76 --- /dev/null +++ b/server/jellyfin/discovery_test.go @@ -0,0 +1,175 @@ +package jellyfin + +import ( + "context" + "encoding/json" + "errors" + "net" + "os" + "time" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +type failingConn struct { + net.PacketConn + closed bool +} + +func (c *failingConn) ReadFrom([]byte) (int, net.Addr, error) { + return 0, nil, errors.New("read failed") +} + +func (c *failingConn) Close() error { + c.closed = true + return nil +} + +var _ = Describe("Discovery", func() { + var d *Discovery + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.Jellyfin.ServerName = "Test Server" + conf.Server.Address = "0.0.0.0" + conf.Server.Port = 4533 + conf.Server.BaseHost = "" + conf.Server.BaseScheme = "" + conf.Server.BasePath = "" + conf.Server.TLSCert = "" + conf.Server.TLSKey = "" + d = &Discovery{} + }) + + DescribeTable("discoveryAddress", + func(setup func(), expected string) { + setup() + remote := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 50000} + Expect(discoveryAddress(context.Background(), remote)).To(Equal(expected)) + }, + Entry("uses the BaseURL host and scheme when set", func() { + conf.Server.BaseScheme = "https" + conf.Server.BaseHost = "music.example.com" + conf.Server.BasePath = "/nd" + }, "https://music.example.com/nd/jellyfin"), + Entry("uses a specific bind Address with the Port", func() { + conf.Server.Address = "192.168.1.10" + }, "http://192.168.1.10:4533/jellyfin"), + Entry("falls back to the interface facing the requester when Address is unspecified", func() {}, + "http://127.0.0.1:4533/jellyfin"), + Entry("falls back to the interface facing the requester when Address is empty", func() { + conf.Server.Address = "" + }, "http://127.0.0.1:4533/jellyfin"), + Entry("advertises https when TLS is configured", func() { + conf.Server.TLSCert = "/path/cert.pem" + conf.Server.TLSKey = "/path/key.pem" + }, "https://127.0.0.1:4533/jellyfin"), + Entry("advertises http when only the TLS cert is configured", func() { + conf.Server.TLSCert = "/path/cert.pem" + }, "http://127.0.0.1:4533/jellyfin"), + Entry("keeps a path-only BaseURL as the path prefix", func() { + conf.Server.BasePath = "/music" + }, "http://127.0.0.1:4533/music/jellyfin"), + Entry("does not double the slash when BasePath has a trailing slash", func() { + conf.Server.BasePath = "/music/" + }, "http://127.0.0.1:4533/music/jellyfin"), + ) + + DescribeTable("hasAdvertisableAddress", + func(address, baseHost string, expected bool) { + conf.Server.Address = address + conf.Server.BaseHost = baseHost + Expect(hasAdvertisableAddress()).To(Equal(expected)) + }, + Entry("TCP listener", "0.0.0.0", "", true), + Entry("unix socket without a BaseURL host", "unix:/tmp/navidrome.sock", "", false), + Entry("unix socket behind a proxy named by BaseURL", "unix:/tmp/navidrome.sock", "music.example.com", true), + ) + + It("closes the connection when the read loop fails", func() { + fake := &failingConn{} + d.ServeOn(context.Background(), fake) + Expect(fake.closed).To(BeTrue()) + }) + + Describe("ServeOn", func() { + var ( + server net.PacketConn + client net.PacketConn + cancel context.CancelFunc + done chan struct{} + ) + + BeforeEach(func() { + var err error + server, err = net.ListenPacket("udp4", "127.0.0.1:0") + Expect(err).ToNot(HaveOccurred()) + client, err = net.ListenPacket("udp4", "127.0.0.1:0") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(client.Close) + + var ctx context.Context + ctx, cancel = context.WithCancel(context.Background()) + done = make(chan struct{}) + go func() { + defer close(done) + d.ServeOn(ctx, server) + }() + DeferCleanup(func() { + cancel() + Eventually(done).Should(BeClosed()) + }) + }) + + send := func(msg string) { + _, err := client.WriteTo([]byte(msg), server.LocalAddr()) + Expect(err).ToNot(HaveOccurred()) + } + receive := func() ([]byte, error) { + Expect(client.SetReadDeadline(time.Now().Add(time.Second))).To(Succeed()) + buf := make([]byte, 1024) + n, _, err := client.ReadFrom(buf) + return buf[:n], err + } + + It("answers the discovery query with the server identity", func() { + send("who is JellyfinServer?") + res, err := receive() + Expect(err).ToNot(HaveOccurred()) + + var info discoveryInfo + Expect(json.Unmarshal(res, &info)).To(Succeed()) + Expect(info.Address).To(Equal("http://127.0.0.1:4533/jellyfin")) + Expect(info.Id).To(Equal(d.serverID(context.Background()))) + Expect(info.Name).To(Equal("Test Server")) + Expect(string(res)).To(ContainSubstring(`"EndpointAddress":null`)) + }) + + It("matches the query case-insensitively", func() { + send("WHO IS JELLYFINSERVER?") + _, err := receive() + Expect(err).ToNot(HaveOccurred()) + }) + + // The loop is serial, so a reply to the first packet would arrive before the second's. + It("ignores unrelated packets", func() { + send("who is PlexServer?") + send("who is JellyfinServer?") + _, err := receive() + Expect(err).ToNot(HaveOccurred()) + Expect(client.SetReadDeadline(time.Now().Add(50 * time.Millisecond))).To(Succeed()) + _, _, err = client.ReadFrom(make([]byte, 1024)) + Expect(errors.Is(err, os.ErrDeadlineExceeded)).To(BeTrue()) + }) + + It("stops and closes the socket when the context is cancelled", func() { + cancel() + Eventually(done).Should(BeClosed()) + _, _, err := server.ReadFrom(make([]byte, 1)) + Expect(err).To(HaveOccurred()) + }) + }) +}) diff --git a/server/jellyfin/e2e/discovery_test.go b/server/jellyfin/e2e/discovery_test.go new file mode 100644 index 000000000..0126ef3c6 --- /dev/null +++ b/server/jellyfin/e2e/discovery_test.go @@ -0,0 +1,50 @@ +package e2e + +import ( + "context" + "encoding/json" + "net" + "time" + + "github.com/navidrome/navidrome/server/jellyfin" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Auto discovery", func() { + BeforeEach(func() { setupTestDB() }) + + It("advertises the same Id and Name as /System/Info/Public", func() { + server, err := net.ListenPacket("udp4", "127.0.0.1:0") + Expect(err).ToNot(HaveOccurred()) + client, err := net.ListenPacket("udp4", "127.0.0.1:0") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(client.Close) + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + jellyfin.NewDiscovery(ds).ServeOn(ctx, server) + }() + DeferCleanup(func() { + cancel() + Eventually(done).Should(BeClosed()) + }) + + _, err = client.WriteTo([]byte("who is JellyfinServer?"), server.LocalAddr()) + Expect(err).ToNot(HaveOccurred()) + Expect(client.SetReadDeadline(time.Now().Add(time.Second))).To(Succeed()) + buf := make([]byte, 1024) + n, _, err := client.ReadFrom(buf) + Expect(err).ToNot(HaveOccurred()) + var reply map[string]any + Expect(json.Unmarshal(buf[:n], &reply)).To(Succeed()) + + var pub map[string]any + parseInto(rawReq("GET", "/System/Info/Public", ""), &pub) + Expect(reply["Id"]).To(Equal(pub["Id"])) + Expect(reply["Name"]).To(Equal(pub["ServerName"])) + Expect(reply["Address"]).To(HaveSuffix("/jellyfin")) + }) +}) diff --git a/server/jellyfin/system.go b/server/jellyfin/system.go index 0a85d4545..35ade5ff1 100644 --- a/server/jellyfin/system.go +++ b/server/jellyfin/system.go @@ -10,6 +10,7 @@ import ( "net/netip" "path" "strings" + "sync" "github.com/google/uuid" "github.com/navidrome/navidrome/conf" @@ -24,35 +25,37 @@ import ( // it (Streamyfin and the Android apps refuse < 10.10), and some parsers need exactly three parts. const jellyfinVersion = "12.1.0" -func (api *Router) serverName() string { +func serverName() string { if conf.Server.Jellyfin.ServerName != "" { return conf.Server.Jellyfin.ServerName } return fmt.Sprintf("Navidrome %s", consts.Version) } -// serverID returns a stable Id that survives restarts, get-or-created in the Property table. -// Jellyfin clients cache ServerId across sessions, so a per-process value would break -// re-authentication. api.ds is nil only in unit tests; New() always sets it. -// -// The mutex serializes first-boot resolution so concurrent requests can't persist different -// UUIDs. Only a successful read or persisted id is cached; a transient failure yields a -// temporary id and retries on the next request rather than pinning a value. func (api *Router) serverID(ctx context.Context) string { - api.serverIDMu.Lock() - defer api.serverIDMu.Unlock() - if api.serverIDVal != "" { - return api.serverIDVal + return resolveServerID(ctx, api.ds, &api.serverIDVal) +} + +// Package-level: the Router and Discovery are separate objects and must not persist different ids. +var serverIDMu sync.Mutex + +// Clients cache ServerId across sessions, so it is persisted. Only a successful read or write is +// cached: a transient failure yields a temporary id and retries on the next call. +func resolveServerID(ctx context.Context, ds model.DataStore, cached *string) string { + serverIDMu.Lock() + defer serverIDMu.Unlock() + if *cached != "" { + return *cached } - if api.ds == nil { - api.serverIDVal = newServerID() - return api.serverIDVal + if ds == nil { + *cached = newServerID() + return *cached } - id, err := api.ds.Property(ctx).Get(consts.JellyfinServerIDKey) + id, err := ds.Property(ctx).Get(consts.JellyfinServerIDKey) switch { case errors.Is(err, model.ErrNotFound): id = newServerID() - if err := api.ds.Property(ctx).Put(consts.JellyfinServerIDKey, id); err != nil { + if err := ds.Property(ctx).Put(consts.JellyfinServerIDKey, id); err != nil { log.Error(ctx, "Jellyfin API: could not persist server id", err) return id } @@ -61,8 +64,8 @@ func (api *Router) serverID(ctx context.Context) string { return newServerID() } // Ids persisted before this change are dashed; normalize on read rather than rewriting the DB. - api.serverIDVal = strings.ReplaceAll(id, "-", "") - return api.serverIDVal + *cached = strings.ReplaceAll(id, "-", "") + return *cached } // newServerID returns a UUID in Jellyfin's no-dash GUID form (Guid.ToString("N")). @@ -74,7 +77,7 @@ func newServerID() string { func (api *Router) publicInfo(r *http.Request) dto.PublicSystemInfo { return dto.PublicSystemInfo{ LocalAddress: localAddress(r), - ServerName: api.serverName(), + ServerName: serverName(), Version: jellyfinVersion, ProductName: "Jellyfin Server", Id: api.serverID(r.Context()), @@ -106,7 +109,7 @@ func (api *Router) getSystemInfo(w http.ResponseWriter, r *http.Request) { func (api *Router) ping(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(api.serverName())) + _, _ = w.Write([]byte(serverName())) } // getEndpointInfo answers /System/Endpoint, which Finamp's connection test uses to pick between a diff --git a/server/jellyfin/system_test.go b/server/jellyfin/system_test.go index 4b3ac240e..c9368f3b5 100644 --- a/server/jellyfin/system_test.go +++ b/server/jellyfin/system_test.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "net/http/httptest" + "sync" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" @@ -173,6 +174,17 @@ var _ = Describe("System", func() { Expect(second.serverID(ctx)).To(Equal(id)) }) + It("resolves one id when a Router and a Discovery race on first boot", func() { + r, d := &Router{ds: ds}, NewDiscovery(ds) + ids := make([]string, 2) + var wg sync.WaitGroup + wg.Go(func() { ids[0] = r.serverID(ctx) }) + wg.Go(func() { ids[1] = d.serverID(ctx) }) + wg.Wait() + Expect(ids[0]).ToNot(BeEmpty()) + Expect(ids[1]).To(Equal(ids[0])) + }) + It("memoizes the id across repeated calls on the same Router", func() { r := &Router{ds: ds} id := r.serverID(ctx) diff --git a/server/jellyfin/users.go b/server/jellyfin/users.go index ee4ace48e..dddcd99da 100644 --- a/server/jellyfin/users.go +++ b/server/jellyfin/users.go @@ -34,7 +34,7 @@ func (api *Router) getUserViews(w http.ResponseWriter, r *http.Request) { func (api *Router) getCurrentUser(w http.ResponseWriter, r *http.Request) { ctx := r.Context() u, _ := request.UserFrom(ctx) - api.ok(w, r, userToDto(&u, api.serverName(), api.serverID(ctx))) + api.ok(w, r, userToDto(&u, serverName(), api.serverID(ctx))) } // getPublicUsers advertises the users named in Jellyfin.ExposedPublicUsers for a client login