feat: use Cover Art Archive for albums with MusicBrainz IDs (#27)
This commit was merged in pull request #27.
This commit is contained in:
Deluan Quintão
GitHub
+246
-9
@@ -5,12 +5,70 @@ import (
|
||||
|
||||
"github.com/navidrome/navidrome/plugins/pdk/go/host"
|
||||
"github.com/navidrome/navidrome/plugins/pdk/go/pdk"
|
||||
"github.com/navidrome/navidrome/plugins/pdk/go/scrobbler"
|
||||
"github.com/stretchr/testify/mock"
|
||||
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("headCoverArt", func() {
|
||||
BeforeEach(func() {
|
||||
pdk.ResetMock()
|
||||
host.HTTPMock.ExpectedCalls = nil
|
||||
host.HTTPMock.Calls = nil
|
||||
pdk.PDKMock.On("Log", mock.Anything, mock.Anything).Maybe()
|
||||
})
|
||||
|
||||
It("returns Location header and definitive=true on 307 response", func() {
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.Method == "HEAD" &&
|
||||
req.URL == "https://coverartarchive.org/release/test-mbid/front-500" &&
|
||||
req.NoFollowRedirects == true
|
||||
})).Return(&host.HTTPResponse{
|
||||
StatusCode: 307,
|
||||
Headers: map[string]string{"Location": "https://archive.org/download/mbid-test/thumb500.jpg"},
|
||||
}, nil)
|
||||
|
||||
result, definitive := headCoverArt("https://coverartarchive.org/release/test-mbid/front-500")
|
||||
Expect(result).To(Equal("https://archive.org/download/mbid-test/thumb500.jpg"))
|
||||
Expect(definitive).To(BeTrue())
|
||||
})
|
||||
|
||||
It("returns empty and definitive=true on 404 response", func() {
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.Method == "HEAD" && req.NoFollowRedirects == true
|
||||
})).Return(&host.HTTPResponse{StatusCode: 404}, nil)
|
||||
|
||||
result, definitive := headCoverArt("https://coverartarchive.org/release/no-art/front-500")
|
||||
Expect(result).To(BeEmpty())
|
||||
Expect(definitive).To(BeTrue())
|
||||
})
|
||||
|
||||
It("returns empty and definitive=false on HTTP error", func() {
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.Method == "HEAD"
|
||||
})).Return((*host.HTTPResponse)(nil), errors.New("connection refused"))
|
||||
|
||||
result, definitive := headCoverArt("https://coverartarchive.org/release/err/front-500")
|
||||
Expect(result).To(BeEmpty())
|
||||
Expect(definitive).To(BeFalse())
|
||||
})
|
||||
|
||||
It("returns empty and definitive=true when Location header is missing on 307", func() {
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.Method == "HEAD"
|
||||
})).Return(&host.HTTPResponse{
|
||||
StatusCode: 307,
|
||||
Headers: map[string]string{},
|
||||
}, nil)
|
||||
|
||||
result, definitive := headCoverArt("https://coverartarchive.org/release/no-location/front-500")
|
||||
Expect(result).To(BeEmpty())
|
||||
Expect(definitive).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("getImageURL", func() {
|
||||
BeforeEach(func() {
|
||||
pdk.ResetMock()
|
||||
@@ -27,40 +85,42 @@ var _ = Describe("getImageURL", func() {
|
||||
|
||||
Describe("uguu disabled (default)", func() {
|
||||
BeforeEach(func() {
|
||||
pdk.PDKMock.On("GetConfig", caaEnabledKey).Return("", false)
|
||||
pdk.PDKMock.On("GetConfig", uguuEnabledKey).Return("", false)
|
||||
})
|
||||
|
||||
It("returns artwork URL directly", func() {
|
||||
host.ArtworkMock.On("GetTrackUrl", "track1", int32(300)).Return("https://example.com/art.jpg", nil)
|
||||
|
||||
url := getImageURL("testuser", "track1")
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1"})
|
||||
Expect(url).To(Equal("https://example.com/art.jpg"))
|
||||
})
|
||||
|
||||
It("returns empty for localhost URL", func() {
|
||||
host.ArtworkMock.On("GetTrackUrl", "track1", int32(300)).Return("http://localhost:4533/art.jpg", nil)
|
||||
|
||||
url := getImageURL("testuser", "track1")
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1"})
|
||||
Expect(url).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("returns empty when artwork fetch fails", func() {
|
||||
host.ArtworkMock.On("GetTrackUrl", "track1", int32(300)).Return("", errors.New("not found"))
|
||||
|
||||
url := getImageURL("testuser", "track1")
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1"})
|
||||
Expect(url).To(BeEmpty())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("uguu enabled", func() {
|
||||
BeforeEach(func() {
|
||||
pdk.PDKMock.On("GetConfig", caaEnabledKey).Return("", false)
|
||||
pdk.PDKMock.On("GetConfig", uguuEnabledKey).Return("true", true)
|
||||
})
|
||||
|
||||
It("returns cached URL when available", func() {
|
||||
host.CacheMock.On("GetString", "uguu.artwork.track1").Return("https://a.uguu.se/cached.jpg", true, nil)
|
||||
|
||||
url := getImageURL("testuser", "track1")
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1"})
|
||||
Expect(url).To(Equal("https://a.uguu.se/cached.jpg"))
|
||||
})
|
||||
|
||||
@@ -78,11 +138,11 @@ var _ = Describe("getImageURL", func() {
|
||||
})).Return(&host.HTTPResponse{StatusCode: 200, Body: []byte(`{"success":true,"files":[{"url":"https://a.uguu.se/uploaded.jpg"}]}`)}, nil)
|
||||
|
||||
// Mock cache set
|
||||
host.CacheMock.On("SetString", "uguu.artwork.track1", "https://a.uguu.se/uploaded.jpg", int64(9000)).Return(nil)
|
||||
host.CacheMock.On("SetString", "uguu.artwork.track1", "https://a.uguu.se/uploaded.jpg", uguuCacheTTL).Return(nil)
|
||||
|
||||
url := getImageURL("testuser", "track1")
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1"})
|
||||
Expect(url).To(Equal("https://a.uguu.se/uploaded.jpg"))
|
||||
host.CacheMock.AssertCalled(GinkgoT(), "SetString", "uguu.artwork.track1", "https://a.uguu.se/uploaded.jpg", int64(9000))
|
||||
host.CacheMock.AssertCalled(GinkgoT(), "SetString", "uguu.artwork.track1", "https://a.uguu.se/uploaded.jpg", uguuCacheTTL)
|
||||
})
|
||||
|
||||
It("returns empty when artwork data fetch fails", func() {
|
||||
@@ -90,7 +150,7 @@ var _ = Describe("getImageURL", func() {
|
||||
host.SubsonicAPIMock.On("CallRaw", "/getCoverArt?u=testuser&id=track1&size=300").
|
||||
Return("", []byte(nil), errors.New("fetch failed"))
|
||||
|
||||
url := getImageURL("testuser", "track1")
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1"})
|
||||
Expect(url).To(BeEmpty())
|
||||
})
|
||||
|
||||
@@ -103,8 +163,185 @@ var _ = Describe("getImageURL", func() {
|
||||
return req.URL == "https://uguu.se/upload"
|
||||
})).Return(&host.HTTPResponse{StatusCode: 500, Body: []byte(`{"success":false}`)}, nil)
|
||||
|
||||
url := getImageURL("testuser", "track1")
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1"})
|
||||
Expect(url).To(BeEmpty())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("CAA enabled", func() {
|
||||
BeforeEach(func() {
|
||||
pdk.PDKMock.ExpectedCalls = nil
|
||||
pdk.PDKMock.On("Log", mock.Anything, mock.Anything).Maybe()
|
||||
pdk.PDKMock.On("GetConfig", caaEnabledKey).Return("true", true)
|
||||
pdk.PDKMock.On("GetConfig", uguuEnabledKey).Return("", false)
|
||||
})
|
||||
|
||||
It("returns CAA URL when release HEAD succeeds", func() {
|
||||
host.CacheMock.On("GetString", "caa.artwork.album-id").Return("", false, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release/album-id/front-500"
|
||||
})).Return(&host.HTTPResponse{
|
||||
StatusCode: 307,
|
||||
Headers: map[string]string{"Location": "https://archive.org/art.jpg"},
|
||||
}, nil)
|
||||
host.CacheMock.On("SetString", "caa.artwork.album-id", "https://archive.org/art.jpg", int64(86400)).Return(nil)
|
||||
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1", MBZAlbumID: "album-id", MBZReleaseGroupID: "rg-id"})
|
||||
Expect(url).To(Equal("https://archive.org/art.jpg"))
|
||||
host.ArtworkMock.AssertNotCalled(GinkgoT(), "GetTrackUrl", mock.Anything, mock.Anything)
|
||||
host.SubsonicAPIMock.AssertNotCalled(GinkgoT(), "CallRaw", mock.Anything)
|
||||
})
|
||||
|
||||
It("falls through to direct when CAA misses and uguu is disabled", func() {
|
||||
host.CacheMock.On("GetString", "caa.artwork.album-id").Return("", false, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release/album-id/front-500"
|
||||
})).Return(&host.HTTPResponse{StatusCode: 404}, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release-group/rg-id/front-500"
|
||||
})).Return(&host.HTTPResponse{StatusCode: 404}, nil)
|
||||
host.CacheMock.On("SetString", "caa.artwork.album-id", "", int64(14400)).Return(nil)
|
||||
host.ArtworkMock.On("GetTrackUrl", "track1", int32(300)).Return("https://example.com/art.jpg", nil)
|
||||
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1", MBZAlbumID: "album-id", MBZReleaseGroupID: "rg-id"})
|
||||
Expect(url).To(Equal("https://example.com/art.jpg"))
|
||||
})
|
||||
|
||||
It("falls through to uguu when CAA misses and uguu is enabled", func() {
|
||||
pdk.PDKMock.ExpectedCalls = nil
|
||||
pdk.PDKMock.On("Log", mock.Anything, mock.Anything).Maybe()
|
||||
pdk.PDKMock.On("GetConfig", caaEnabledKey).Return("true", true)
|
||||
pdk.PDKMock.On("GetConfig", uguuEnabledKey).Return("true", true)
|
||||
|
||||
host.CacheMock.On("GetString", "caa.artwork.rg.rg-id").Return("", false, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release-group/rg-id/front-500"
|
||||
})).Return(&host.HTTPResponse{StatusCode: 404}, nil)
|
||||
host.CacheMock.On("SetString", "caa.artwork.rg.rg-id", "", int64(14400)).Return(nil)
|
||||
|
||||
host.CacheMock.On("GetString", "uguu.artwork.track1").Return("https://a.uguu.se/cached.jpg", true, nil)
|
||||
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1", MBZReleaseGroupID: "rg-id"})
|
||||
Expect(url).To(Equal("https://a.uguu.se/cached.jpg"))
|
||||
})
|
||||
|
||||
It("skips CAA when no MBZ IDs are present", func() {
|
||||
host.ArtworkMock.On("GetTrackUrl", "track1", int32(300)).Return("https://example.com/art.jpg", nil)
|
||||
|
||||
url := getImageURL("testuser", scrobbler.TrackInfo{ID: "track1"})
|
||||
Expect(url).To(Equal("https://example.com/art.jpg"))
|
||||
host.HTTPMock.AssertNotCalled(GinkgoT(), "Send", mock.Anything)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("getImageViaCoverArt", func() {
|
||||
BeforeEach(func() {
|
||||
pdk.ResetMock()
|
||||
host.CacheMock.ExpectedCalls = nil
|
||||
host.CacheMock.Calls = nil
|
||||
host.HTTPMock.ExpectedCalls = nil
|
||||
host.HTTPMock.Calls = nil
|
||||
pdk.PDKMock.On("Log", mock.Anything, mock.Anything).Maybe()
|
||||
})
|
||||
|
||||
It("returns cached URL on cache hit", func() {
|
||||
host.CacheMock.On("GetString", "caa.artwork.album-123").Return("https://archive.org/cached.jpg", true, nil)
|
||||
|
||||
result := getImageViaCoverArt("album-123", "rg-456")
|
||||
Expect(result).To(Equal("https://archive.org/cached.jpg"))
|
||||
host.HTTPMock.AssertNotCalled(GinkgoT(), "Send", mock.Anything)
|
||||
})
|
||||
|
||||
It("returns empty on cache hit with empty string (known miss)", func() {
|
||||
host.CacheMock.On("GetString", "caa.artwork.album-123").Return("", true, nil)
|
||||
|
||||
result := getImageViaCoverArt("album-123", "rg-456")
|
||||
Expect(result).To(BeEmpty())
|
||||
host.HTTPMock.AssertNotCalled(GinkgoT(), "Send", mock.Anything)
|
||||
})
|
||||
|
||||
It("returns release URL on 307 and caches it", func() {
|
||||
host.CacheMock.On("GetString", "caa.artwork.album-123").Return("", false, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release/album-123/front-500"
|
||||
})).Return(&host.HTTPResponse{
|
||||
StatusCode: 307,
|
||||
Headers: map[string]string{"Location": "https://archive.org/release-art.jpg"},
|
||||
}, nil)
|
||||
host.CacheMock.On("SetString", "caa.artwork.album-123", "https://archive.org/release-art.jpg", int64(86400)).Return(nil)
|
||||
|
||||
result := getImageViaCoverArt("album-123", "rg-456")
|
||||
Expect(result).To(Equal("https://archive.org/release-art.jpg"))
|
||||
host.CacheMock.AssertCalled(GinkgoT(), "SetString", "caa.artwork.album-123", "https://archive.org/release-art.jpg", int64(86400))
|
||||
})
|
||||
|
||||
It("falls back to release-group when release returns 404", func() {
|
||||
host.CacheMock.On("GetString", "caa.artwork.album-123").Return("", false, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release/album-123/front-500"
|
||||
})).Return(&host.HTTPResponse{StatusCode: 404}, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release-group/rg-456/front-500"
|
||||
})).Return(&host.HTTPResponse{
|
||||
StatusCode: 307,
|
||||
Headers: map[string]string{"Location": "https://archive.org/rg-art.jpg"},
|
||||
}, nil)
|
||||
host.CacheMock.On("SetString", "caa.artwork.album-123", "https://archive.org/rg-art.jpg", int64(86400)).Return(nil)
|
||||
|
||||
result := getImageViaCoverArt("album-123", "rg-456")
|
||||
Expect(result).To(Equal("https://archive.org/rg-art.jpg"))
|
||||
})
|
||||
|
||||
It("caches empty string when both release and release-group fail", func() {
|
||||
host.CacheMock.On("GetString", "caa.artwork.album-123").Return("", false, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release/album-123/front-500"
|
||||
})).Return(&host.HTTPResponse{StatusCode: 404}, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release-group/rg-456/front-500"
|
||||
})).Return(&host.HTTPResponse{StatusCode: 404}, nil)
|
||||
host.CacheMock.On("SetString", "caa.artwork.album-123", "", int64(14400)).Return(nil)
|
||||
|
||||
result := getImageViaCoverArt("album-123", "rg-456")
|
||||
Expect(result).To(BeEmpty())
|
||||
host.CacheMock.AssertCalled(GinkgoT(), "SetString", "caa.artwork.album-123", "", int64(14400))
|
||||
})
|
||||
|
||||
It("does not cache miss on transient failure", func() {
|
||||
host.CacheMock.On("GetString", "caa.artwork.album-123").Return("", false, nil)
|
||||
// Both requests fail with network errors (transient)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release/album-123/front-500"
|
||||
})).Return((*host.HTTPResponse)(nil), errors.New("connection refused"))
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release-group/rg-456/front-500"
|
||||
})).Return((*host.HTTPResponse)(nil), errors.New("timeout"))
|
||||
|
||||
result := getImageViaCoverArt("album-123", "rg-456")
|
||||
Expect(result).To(BeEmpty())
|
||||
// Should NOT cache the miss since failures were transient
|
||||
host.CacheMock.AssertNotCalled(GinkgoT(), "SetString", mock.Anything, mock.Anything, mock.Anything)
|
||||
})
|
||||
|
||||
It("tries only release-group when MBZAlbumID is empty", func() {
|
||||
host.CacheMock.On("GetString", "caa.artwork.rg.rg-456").Return("", false, nil)
|
||||
host.HTTPMock.On("Send", mock.MatchedBy(func(req host.HTTPRequest) bool {
|
||||
return req.URL == "https://coverartarchive.org/release-group/rg-456/front-500"
|
||||
})).Return(&host.HTTPResponse{
|
||||
StatusCode: 307,
|
||||
Headers: map[string]string{"Location": "https://archive.org/rg-art.jpg"},
|
||||
}, nil)
|
||||
host.CacheMock.On("SetString", "caa.artwork.rg.rg-456", "https://archive.org/rg-art.jpg", int64(86400)).Return(nil)
|
||||
|
||||
result := getImageViaCoverArt("", "rg-456")
|
||||
Expect(result).To(Equal("https://archive.org/rg-art.jpg"))
|
||||
})
|
||||
|
||||
It("returns empty when both IDs are empty", func() {
|
||||
result := getImageViaCoverArt("", "")
|
||||
Expect(result).To(BeEmpty())
|
||||
host.HTTPMock.AssertNotCalled(GinkgoT(), "Send", mock.Anything)
|
||||
host.CacheMock.AssertNotCalled(GinkgoT(), "GetString", mock.Anything)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user