diff --git a/rpc.go b/rpc.go index a75c5e7..3305039 100644 --- a/rpc.go +++ b/rpc.go @@ -24,6 +24,12 @@ const ( const heartbeatInterval = 41 // Heartbeat interval in seconds +// Image cache TTL constants +const ( + imageCacheTTL int64 = 4 * 60 * 60 // 4 hours for track artwork + defaultImageCacheTTL int64 = 48 * 60 * 60 // 48 hours for default Navidrome logo +) + // Scheduler callback payloads for routing const ( payloadHeartbeat = "heartbeat" @@ -112,13 +118,11 @@ type identifyProperties struct { // Image Processing // ============================================================================ -// processImage processes an image URL for Discord, with fallback to default image. -func (r *discordRPC) processImage(imageURL, clientID, token string, isDefaultImage bool) (string, error) { +// processImage processes an image URL for Discord. Returns the processed image +// string (mp:prefixed) or an error. No fallback logic — the caller handles retries. +func (r *discordRPC) processImage(imageURL, clientID, token string, ttl int64) (string, error) { if imageURL == "" { - if isDefaultImage { - return "", fmt.Errorf("default image URL is empty") - } - return r.processImage(navidromeLogoURL, clientID, token, true) + return "", fmt.Errorf("image URL is empty") } if strings.HasPrefix(imageURL, "mp:") { @@ -142,43 +146,25 @@ func (r *discordRPC) processImage(imageURL, clientID, token string, isDefaultIma resp := req.Send() if resp.Status() >= 400 { - if isDefaultImage { - return "", fmt.Errorf("failed to process default image: HTTP %d", resp.Status()) - } - return r.processImage(navidromeLogoURL, clientID, token, true) + return "", fmt.Errorf("failed to process image: HTTP %d", resp.Status()) } var data []map[string]string if err := json.Unmarshal(resp.Body(), &data); err != nil { - if isDefaultImage { - return "", fmt.Errorf("failed to unmarshal default image response: %w", err) - } - return r.processImage(navidromeLogoURL, clientID, token, true) + return "", fmt.Errorf("failed to unmarshal image response: %w", err) } if len(data) == 0 { - if isDefaultImage { - return "", fmt.Errorf("no data returned for default image") - } - return r.processImage(navidromeLogoURL, clientID, token, true) + return "", fmt.Errorf("no data returned for image") } image := data[0]["external_asset_path"] if image == "" { - if isDefaultImage { - return "", fmt.Errorf("empty external_asset_path for default image") - } - return r.processImage(navidromeLogoURL, clientID, token, true) + return "", fmt.Errorf("empty external_asset_path for image") } processedImage := fmt.Sprintf("mp:%s", image) - // Cache the processed image URL - var ttl int64 = 4 * 60 * 60 // 4 hours for regular images - if isDefaultImage { - ttl = 48 * 60 * 60 // 48 hours for default image - } - _ = host.CacheSetString(cacheKey, processedImage, ttl) pdk.Log(pdk.LogDebug, fmt.Sprintf("Cached processed image URL for %s (TTL: %ds)", imageURL, ttl)) @@ -193,19 +179,33 @@ func (r *discordRPC) processImage(imageURL, clientID, token string, isDefaultIma func (r *discordRPC) sendActivity(clientID, username, token string, data activity) error { pdk.Log(pdk.LogInfo, fmt.Sprintf("Sending activity for user %s: %s - %s", username, data.Details, data.State)) - processedImage, err := r.processImage(data.Assets.LargeImage, clientID, token, false) + // Try track artwork first, fall back to Navidrome logo + usingDefaultImage := false + processedImage, err := r.processImage(data.Assets.LargeImage, clientID, token, imageCacheTTL) if err != nil { - pdk.Log(pdk.LogWarn, fmt.Sprintf("Failed to process image for user %s, continuing without image: %v", username, err)) - data.Assets.LargeImage = "" + pdk.Log(pdk.LogWarn, fmt.Sprintf("Failed to process track image for user %s: %v, falling back to default", username, err)) + processedImage, err = r.processImage(navidromeLogoURL, clientID, token, defaultImageCacheTTL) + if err != nil { + pdk.Log(pdk.LogWarn, fmt.Sprintf("Failed to process default image for user %s: %v, continuing without image", username, err)) + data.Assets.LargeImage = "" + } else { + data.Assets.LargeImage = processedImage + usingDefaultImage = true + } } else { data.Assets.LargeImage = processedImage } - if data.Assets.SmallImage != "" { - processedSmall, err := r.processImage(data.Assets.SmallImage, clientID, token, false) + // Only show SmallImage (Navidrome logo overlay) when LargeImage is actual track artwork + if usingDefaultImage || data.Assets.LargeImage == "" { + data.Assets.SmallImage = "" + data.Assets.SmallText = "" + } else if data.Assets.SmallImage != "" { + processedSmall, err := r.processImage(data.Assets.SmallImage, clientID, token, defaultImageCacheTTL) if err != nil { pdk.Log(pdk.LogWarn, fmt.Sprintf("Failed to process small image for user %s: %v", username, err)) data.Assets.SmallImage = "" + data.Assets.SmallText = "" } else { data.Assets.SmallImage = processedSmall } diff --git a/rpc_test.go b/rpc_test.go index b85c27e..258e9c9 100644 --- a/rpc_test.go +++ b/rpc_test.go @@ -232,26 +232,115 @@ var _ = Describe("discordRPC", func() { }) }) + Describe("processImage", func() { + BeforeEach(func() { + pdk.PDKMock.On("Log", mock.Anything, mock.Anything).Maybe() + }) + + It("returns error for empty URL", func() { + _, err := r.processImage("", "client123", "token123", imageCacheTTL) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("image URL is empty")) + }) + + It("returns mp: prefixed URL as-is", func() { + result, err := r.processImage("mp:external/abc123", "client123", "token123", imageCacheTTL) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal("mp:external/abc123")) + }) + + It("returns cached value on cache hit", func() { + host.CacheMock.On("GetString", mock.MatchedBy(func(key string) bool { + return strings.HasPrefix(key, "discord.image.") + })).Return("mp:cached/image", true, nil) + + result, err := r.processImage("https://example.com/art.jpg", "client123", "token123", imageCacheTTL) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal("mp:cached/image")) + }) + + It("processes image via Discord API and caches result", func() { + host.CacheMock.On("GetString", mock.Anything).Return("", false, nil) + host.CacheMock.On("SetString", mock.Anything, mock.MatchedBy(func(val string) bool { + return val == "mp:external/new-asset" + }), int64(imageCacheTTL)).Return(nil) + + httpReq := &pdk.HTTPRequest{} + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(httpReq) + pdk.PDKMock.On("Send", mock.Anything).Return(pdk.NewStubHTTPResponse(200, nil, []byte(`[{"external_asset_path":"external/new-asset"}]`))) + + result, err := r.processImage("https://example.com/art.jpg", "client123", "token123", imageCacheTTL) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal("mp:external/new-asset")) + }) + + It("returns error on HTTP failure", func() { + host.CacheMock.On("GetString", mock.Anything).Return("", false, nil) + + httpReq := &pdk.HTTPRequest{} + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(httpReq) + pdk.PDKMock.On("Send", mock.Anything).Return(pdk.NewStubHTTPResponse(500, nil, []byte(`error`))) + + _, err := r.processImage("https://example.com/art.jpg", "client123", "token123", imageCacheTTL) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("HTTP 500")) + }) + + It("returns error on unmarshal failure", func() { + host.CacheMock.On("GetString", mock.Anything).Return("", false, nil) + + httpReq := &pdk.HTTPRequest{} + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(httpReq) + pdk.PDKMock.On("Send", mock.Anything).Return(pdk.NewStubHTTPResponse(200, nil, []byte(`{"not":"an-array"}`))) + + _, err := r.processImage("https://example.com/art.jpg", "client123", "token123", imageCacheTTL) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to unmarshal")) + }) + + It("returns error on empty response array", func() { + host.CacheMock.On("GetString", mock.Anything).Return("", false, nil) + + httpReq := &pdk.HTTPRequest{} + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(httpReq) + pdk.PDKMock.On("Send", mock.Anything).Return(pdk.NewStubHTTPResponse(200, nil, []byte(`[]`))) + + _, err := r.processImage("https://example.com/art.jpg", "client123", "token123", imageCacheTTL) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no data returned")) + }) + + It("returns error on empty external_asset_path", func() { + host.CacheMock.On("GetString", mock.Anything).Return("", false, nil) + + httpReq := &pdk.HTTPRequest{} + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(httpReq) + pdk.PDKMock.On("Send", mock.Anything).Return(pdk.NewStubHTTPResponse(200, nil, []byte(`[{"external_asset_path":""}]`))) + + _, err := r.processImage("https://example.com/art.jpg", "client123", "token123", imageCacheTTL) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("empty external_asset_path")) + }) + }) + Describe("sendActivity", func() { BeforeEach(func() { pdk.PDKMock.On("Log", mock.Anything, mock.Anything).Maybe() - host.CacheMock.On("GetString", mock.MatchedBy(func(key string) bool { - return strings.HasPrefix(key, "discord.image.") - })).Return("", false, nil) - host.CacheMock.On("SetString", mock.Anything, mock.Anything, mock.Anything).Return(nil) - - // Mock HTTP request for Discord external assets API (image processing) - // When processImage is called, it makes an HTTP request - httpReq := &pdk.HTTPRequest{} - pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(httpReq) - pdk.PDKMock.On("Send", mock.Anything).Return(pdk.NewStubHTTPResponse(200, nil, []byte(`{"key":"test-key"}`))) }) - It("sends activity update to Discord", func() { + It("sends activity with track artwork and SmallImage overlay", func() { + host.CacheMock.On("GetString", mock.Anything).Return("", false, nil) + host.CacheMock.On("SetString", mock.Anything, mock.Anything, mock.Anything).Return(nil) + + httpReq := &pdk.HTTPRequest{} + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(httpReq) + pdk.PDKMock.On("Send", mock.Anything).Return(pdk.NewStubHTTPResponse(200, nil, []byte(`[{"external_asset_path":"external/art"}]`))) + host.WebSocketMock.On("SendText", "testuser", mock.MatchedBy(func(msg string) bool { return strings.Contains(msg, `"op":3`) && - strings.Contains(msg, `"name":"Test Song"`) && - strings.Contains(msg, `"state":"Test Artist"`) + strings.Contains(msg, `"large_image":"mp:external/art"`) && + strings.Contains(msg, `"small_image":"mp:external/art"`) && + strings.Contains(msg, `"small_text":"Navidrome"`) })).Return(nil) err := r.sendActivity("client123", "testuser", "token123", activity{ @@ -260,6 +349,113 @@ var _ = Describe("discordRPC", func() { Type: 2, State: "Test Artist", Details: "Test Album", + Assets: activityAssets{ + LargeImage: "https://example.com/art.jpg", + LargeText: "Test Album", + SmallImage: navidromeLogoURL, + SmallText: "Navidrome", + }, + }) + Expect(err).ToNot(HaveOccurred()) + }) + + It("falls back to default image and clears SmallImage", func() { + // Track art fails (HTTP error), default image succeeds + host.CacheMock.On("GetString", mock.Anything).Return("", false, nil) + host.CacheMock.On("SetString", mock.Anything, mock.Anything, mock.Anything).Return(nil) + + trackReq := &pdk.HTTPRequest{} + defaultReq := &pdk.HTTPRequest{} + + // First call (track art) returns 500, second call (default) succeeds + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(trackReq).Once() + pdk.PDKMock.On("Send", trackReq).Return(pdk.NewStubHTTPResponse(500, nil, []byte(`error`))).Once() + + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(defaultReq).Once() + pdk.PDKMock.On("Send", defaultReq).Return(pdk.NewStubHTTPResponse(200, nil, []byte(`[{"external_asset_path":"external/logo"}]`))).Once() + + host.WebSocketMock.On("SendText", "testuser", mock.MatchedBy(func(msg string) bool { + return strings.Contains(msg, `"op":3`) && + strings.Contains(msg, `"large_image":"mp:external/logo"`) && + !strings.Contains(msg, `"small_image":"mp:`) && + !strings.Contains(msg, `"small_text":"Navidrome"`) + })).Return(nil) + + err := r.sendActivity("client123", "testuser", "token123", activity{ + Application: "client123", + Name: "Test Song", + Type: 2, + State: "Test Artist", + Details: "Test Album", + Assets: activityAssets{ + LargeImage: "https://example.com/art.jpg", + LargeText: "Test Album", + SmallImage: navidromeLogoURL, + SmallText: "Navidrome", + }, + }) + Expect(err).ToNot(HaveOccurred()) + }) + + It("clears all images when both track art and default fail", func() { + host.CacheMock.On("GetString", mock.Anything).Return("", false, nil) + + httpReq := &pdk.HTTPRequest{} + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(httpReq) + pdk.PDKMock.On("Send", mock.Anything).Return(pdk.NewStubHTTPResponse(200, nil, []byte(`{"not":"array"}`))) + + host.WebSocketMock.On("SendText", "testuser", mock.MatchedBy(func(msg string) bool { + return strings.Contains(msg, `"op":3`) && + strings.Contains(msg, `"large_image":""`) && + !strings.Contains(msg, `"small_image":"mp:`) + })).Return(nil) + + err := r.sendActivity("client123", "testuser", "token123", activity{ + Application: "client123", + Name: "Test Song", + Type: 2, + State: "Test Artist", + Details: "Test Album", + Assets: activityAssets{ + LargeImage: "https://example.com/art.jpg", + LargeText: "Test Album", + SmallImage: navidromeLogoURL, + SmallText: "Navidrome", + }, + }) + Expect(err).ToNot(HaveOccurred()) + }) + + It("handles SmallImage processing failure gracefully", func() { + // LargeImage from cache (succeeds), SmallImage API fails + host.CacheMock.On("GetString", mock.MatchedBy(func(key string) bool { + return strings.HasPrefix(key, "discord.image.") + })).Return("mp:cached/large", true, nil).Once() + host.CacheMock.On("GetString", mock.MatchedBy(func(key string) bool { + return strings.HasPrefix(key, "discord.image.") + })).Return("", false, nil).Once() + + httpReq := &pdk.HTTPRequest{} + pdk.PDKMock.On("NewHTTPRequest", pdk.MethodPost, mock.Anything).Return(httpReq) + pdk.PDKMock.On("Send", mock.Anything).Return(pdk.NewStubHTTPResponse(500, nil, []byte(`error`))) + + host.WebSocketMock.On("SendText", "testuser", mock.MatchedBy(func(msg string) bool { + return strings.Contains(msg, `"large_image":"mp:cached/large"`) && + !strings.Contains(msg, `"small_image":"mp:`) + })).Return(nil) + + err := r.sendActivity("client123", "testuser", "token123", activity{ + Application: "client123", + Name: "Test Song", + Type: 2, + State: "Test Artist", + Details: "Test Album", + Assets: activityAssets{ + LargeImage: "https://example.com/art.jpg", + LargeText: "Test Album", + SmallImage: navidromeLogoURL, + SmallText: "Navidrome", + }, }) Expect(err).ToNot(HaveOccurred()) })