mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
7a11ca69bb
* fix(jellyfin): honor Filters=IsFavorite on /Artists and /Artists/AlbumArtists listArtistsByRole hand-built its itemsQuery and never set favOnly, so the favorites filter was silently dropped on both artist routes while /Items honored it. Finamp's home screen asks for favorite artists once per load and was served the entire artist list instead: 10,298 artists, 6.15 MB, 2.7s on a real library, and the wrong data on screen. Extract the favOnly parsing that parseItemsQuery already did into parseFavOnly and use it in both places. listArtists now adds the starred predicate to notMissing rather than replacing it, matching listAlbums and listSongs, so a favorite artist whose files are gone stays excluded. * fix(jellyfin): map SortBy=Runtime to duration for albums and songs sortColumnsByType had no runtime/runtimeticks key for any type, so Finamp's "Duration" sort silently misbehaved in two different ways. Albums: Finamp sends a bare SortBy=Runtime. Nothing matched, opts.Sort stayed empty, and applyOptions skips OrderBy entirely when Sort is empty — so the query ran with no ORDER BY at all and Ascending and Descending returned identical lists. Songs: Finamp sends SortBy=Runtime,AlbumArtist,Album,SortName. applySort takes the first *recognized* key, so Runtime was skipped and the list came back sorted by album artist while looking correct. Both repos already accept a duration sort (mediafile_repository maps it explicitly; album_repository falls through to the column name), so no migration is needed. Sorting 97k songs by duration costs a temp B-tree (~114ms on a prod-sized copy) — the same cost the Subsonic and UI duration sorts already pay, and correct where the previous behaviour was merely fast. * fix(jellyfin): apply the played/unplayed filters and MaxHeight image bound Filters was matched with a substring test for IsFavorite, so every other token Jellyfin defines was silently dropped and the response kept rows it should have excluded. Finamp sends Filters=IsUnplayed in normal use. Replace the bool with a parsed itemFilters carrying nullable favorite and played flags, so isFavorite=false and isPlayed=false are real filters rather than indistinguishable from an absent param. Standalone params are read first and the Filters list overrides them, the precedence real Jellyfin has. IsFavoriteOrLikes now maps to favorites deliberately instead of by substring accident; Likes, Dislikes, IsFolder, IsNotFolder and IsResumable have no Navidrome equivalent and are dropped rather than half-applied. The negative cases match NULL as well, since annotations are LEFT JOINed and an untouched item has no row. getItemImage read only maxwidth, so a client sending just MaxHeight got the full-size original: measured against a real cover, maxHeight=100 returned 82,570 bytes where maxWidth=100 returned 3,316. Use the tighter of the two bounds. * refactor(jellyfin): share the plain-param parser between /Items and /Artists listArtistsByRole hand-listed the itemsQuery fields it happened to need, which is exactly how the favorites filter went missing: the literal has been amended in four of the five commits that touched it. Extract listParams for the fields that come straight from query params so both paths read one parser, and the next supported param reaches every list path instead of only /Items. Also from the cleanup pass: collapse imageSize to a single clamped comparison and read its bounds through req.Params like the rest of the package, which drops the strconv import; build the artist and playlist filter lists with the flat append shape the album and song paths already use, instead of re-wrapping opts.Filters into a nested And per predicate; drop a nil guard in listPlaylists that no caller can reach, since both paths into queryItemsOfType build QueryOptions without Filters. applySort now logs when no SortBy key resolves at all — a miss inside a fallback list is normal, but none matching means a silently ignored sort, the failure mode that hid the Runtime bug. Its doc comment records why the remaining keys cannot simply be joined. Folds three duplicated test bodies into the tables that already parameterize them, and covers the artist-parent album branch, which reaches notMissing through filter.AlbumsByArtistID rather than the default branch. * docs(jellyfin): correct how applySort describes Jellyfin's SortBy semantics The comment claimed SortBy is a comma-separated fallback list. It is not: RequestHelpers.GetOrderBy (10.10) builds one (ItemSortBy, SortOrder) pair per key, so Jellyfin orders by every key in turn. Navidrome applies only the first recognized one, which is a real divergence — secondary keys never break ties — not the intended reading of the parameter. The assertion that the keys cannot be joined was also wrong. buildSortOrder does split its input on commas; what it maps is the whole string, so joining raw Jellyfin key names misses the mappings. Mapping each key first and joining the results would work, which makes multi-key sorting a real option rather than a blocked one. Documenting the current behaviour as a known divergence until then. * fix(jellyfin): order by every recognized SortBy key, not just the first Jellyfin orders by each SortBy key in turn, so "DatePlayed,SortName" means break ties by name. Navidrome applied only the first recognized key and dropped the rest, which is 28% of the sort traffic on a real server (23 of 82 requests in 12h carry 2-5 keys). Most were harmless because the primary key dominates, but PremiereDate,Album,ParentIndexNumber,IndexNumber,SortName came back unordered within a year. The keys cannot simply be joined: sortMapping keyed on the whole Sort string, so a joined value missed every mapping and fell through to raw column names. Make it resolve a comma list per part, but only when every part is a known key — the four existing callers that pass raw column lists (core/matcher, core/lyrics, core/maintenance, subsonic/browsing) all carry a part that is not a mapping key, several with their own direction, so they keep falling through exactly as before. Verified each one. applySort now collects every recognized key, skipping duplicates so ParentIndexNumber,IndexNumber does not repeat a column. random stays alone: the repo matches it by exact string equality, so joining it would both break that path and emit a bare 'random' column into the ORDER BY. Verified against a prod-sized copy: every multi-key combination seen in real traffic returns 200, and a secondary key now changes the order within a tied year for songs. Albums are unchanged there, because their max_year mapping already ended in ", name". * fix(persistence): resolve sort mappings exactly once Making sortMapping resolve a comma list per part broke an invariant it had been relying on: idempotence. sanitizeSort mapped the sort key up front and applyOptions then ran buildSortOrder over the result, so sortMapping was already being handed its own output. That was harmless only while a mapped value could never look like a key list. media_file's rated_at maps to "rating, rated_at", and both parts are keys, so the second pass expanded it to "rating, rating, rated_at". Found by round-tripping every mapping in all four repositories; it was the only collision, and the duplicate sort key was benign in SQL, but any future mapping of that shape would silently change meaning. sanitizeSort now validates without resolving, leaving buildSortOrder as the single mapping point. The generated SQL is unchanged — the whole suite passes apart from the two specs that asserted the old return value, which are updated and joined by a round-trip guard covering exactly the rated_at shape. Also use the paren-aware splitFunc that buildSortOrder already uses, so an expression carrying commas inside its parentheses cannot be split apart. * refactor(jellyfin,persistence): flatten the sort resolution paths Cleanup pass over the branch, no behavior change. sortMapping loses the len(parts)>1 guard, which existed only to pick between two identical toSnakeCase exits; the single-key case now falls through the same loop. lookupSortMapping hands back the snake_case form it had to derive so the fallback stops recomputing it — toSnakeCase is two regexps, and on a miss it was running twice per call. sanitizeSort now asks lookupSortMapping instead of probing the map itself, so "is this a known sort key" has one answer; the two had already drifted, since sanitizeSort tried one casing where the resolver tries three. applySort folds the nested random branch into the skip condition and the two trailing length tests into one switch. setSortMappings documents the invariant the comma-list rule depends on, where someone adding a mapping will read it. The README line describing SortBy still said only the first key applied, which the commit before last made false. Tests: the twelve near-identical sorting specs become one DescribeTable of (itemType, SortBy, want) triples, 124 lines to 36, and the applyOptions round-trip assertion collapses to the buildSortOrder call its sibling uses. * fix(jellyfin): keep annotation filters out of search, resolve sorts per part Two findings from the Codex review on #5981. The played/unplayed filters turned working requests into 500s when combined with SearchTerm. Search runs a two-phase FTS query whose first phase selects rowids with no annotation join, so a starred or play_count predicate there is "no such column", not a filter. Measured against master: MusicAlbum with SearchTerm and Filters=IsUnplayed went 200 -> 500, likewise IsPlayed and the Audio equivalents. listAlbums and listSongs now skip those predicates on the search path, matching what listArtists already did. That also clears the same 500 master already had for Filters=IsFavorite with SearchTerm. sortMapping resolved a comma list only while every part was a known key, so a list mixing a plain column with a mapped key kept neither: MusicAlbum SortBy=Runtime,SortName arrives as "duration, name", and duration is a plain album column, so name stayed raw instead of expanding to order_album_name. Albums whose name differs from its sort form — 1,366 of 6,987 on a real library — then ordered by the wrong secondary key, and PreferSortTags was ignored. Each part is now resolved on its own, which is what setSortMappings already documents for a single field. Verified every in-tree caller that passes a raw column list still produces its original ORDER BY. Codex also asked for the artist search path to apply the same filters. It would 500 for the reason above, and wrapping the library scope in a compound filter makes requestedLibraryIDs stop recognizing it, silently widening the search past the requested ParentId. * fix(jellyfin): honor the first SortOrder value for a multi-key sort applySort compared the whole SortOrder string with "Descending", so a per-key list like SortOrder=Descending,Ascending failed the match and every key, including the primary, sorted ascending — the exact opposite of the request. Take the first comma-separated value, which Jellyfin also uses for any key past the end of the SortOrder list. True per-key directions can't be expressed through the single opts.Sort string and are left out; no observed client sends a SortOrder list.
471 lines
17 KiB
Go
471 lines
17 KiB
Go
package jellyfin
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/base64"
|
|
"errors"
|
|
"image"
|
|
"image/gif"
|
|
"image/jpeg"
|
|
"image/png"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strconv"
|
|
"strings"
|
|
"time"
|
|
|
|
"github.com/go-chi/chi/v5"
|
|
"github.com/navidrome/navidrome/conf"
|
|
"github.com/navidrome/navidrome/conf/configtest"
|
|
"github.com/navidrome/navidrome/core/artwork"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/model/request"
|
|
"github.com/navidrome/navidrome/server/jellyfin/dto"
|
|
"github.com/navidrome/navidrome/tests"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
. "github.com/onsi/gomega"
|
|
)
|
|
|
|
type fakeArtwork struct {
|
|
artwork.Artwork
|
|
recvId string
|
|
recvSize int
|
|
recvCtx context.Context
|
|
data []byte
|
|
hash string
|
|
}
|
|
|
|
func (f *fakeArtwork) GetOrPlaceholder(ctx context.Context, id string, size int, square bool) (*artwork.Image, error) {
|
|
f.recvId = id
|
|
f.recvSize = size
|
|
f.recvCtx = ctx
|
|
data := f.data
|
|
if data == nil {
|
|
data = []byte("IMG")
|
|
}
|
|
return &artwork.Image{
|
|
ReadCloser: io.NopCloser(bytes.NewReader(data)),
|
|
Hash: f.hash,
|
|
LastUpdated: time.Now(),
|
|
}, nil
|
|
}
|
|
|
|
func newImageRequest(itemId string) (*httptest.ResponseRecorder, *http.Request) {
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("GET", "/Items/"+itemId+"/Images/Primary", nil)
|
|
rctx := chi.NewRouteContext()
|
|
rctx.URLParams.Add("itemId", itemId)
|
|
rctx.URLParams.Add("type", "Primary")
|
|
r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx))
|
|
return w, r
|
|
}
|
|
|
|
var _ = Describe("Images", func() {
|
|
// Real Jellyfin fits the image inside either bound, so a client that sends only MaxHeight must
|
|
// still get a resized image rather than the full-size original.
|
|
DescribeTable("derives the requested size from MaxWidth or MaxHeight",
|
|
func(query string, wantSize int) {
|
|
ds := &tests.MockDataStore{}
|
|
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
|
|
fa := &fakeArtwork{}
|
|
api := &Router{ds: ds, artwork: fa}
|
|
|
|
w, r := newImageRequest(dto.EncodeID(testID("a1")))
|
|
r.URL.RawQuery = query
|
|
api.getItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
Expect(fa.recvSize).To(Equal(wantSize))
|
|
},
|
|
Entry("MaxWidth only", "maxwidth=300", 300),
|
|
Entry("MaxHeight only", "maxheight=300", 300),
|
|
Entry("both, smaller bound wins", "maxwidth=200&maxheight=300", 200),
|
|
Entry("both, smaller bound wins regardless of order", "maxwidth=300&maxheight=200", 200),
|
|
Entry("neither", "", 0),
|
|
)
|
|
|
|
It("streams album artwork", func() {
|
|
ds := &tests.MockDataStore{}
|
|
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
|
|
fa := &fakeArtwork{}
|
|
api := &Router{ds: ds, artwork: fa}
|
|
|
|
w, r := newImageRequest(dto.EncodeID(testID("a1")))
|
|
api.getItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
Expect(w.Body.String()).To(Equal("IMG"))
|
|
Expect(fa.recvId).To(ContainSubstring(testID("a1")))
|
|
})
|
|
|
|
// A malformed itemId now 404s via itemIDParam instead of falling through to a placeholder image.
|
|
It("404s a malformed itemId instead of serving a placeholder", func() {
|
|
ds := &tests.MockDataStore{}
|
|
fa := &fakeArtwork{}
|
|
api := &Router{ds: ds, artwork: fa}
|
|
|
|
w, r := newImageRequest("not-a-valid-id")
|
|
api.getItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNotFound))
|
|
Expect(fa.recvId).To(BeEmpty(), "artwork resolution must not run for an undecodable id")
|
|
})
|
|
|
|
// resolveArtworkID probes the entity tables, so a deleted item yields no artwork id at all.
|
|
It("asks for no artwork once the item is deleted, rather than its lingering state", func() {
|
|
ds := &tests.MockDataStore{} // no albums/artists/tracks/playlists at all
|
|
fa := &fakeArtwork{}
|
|
api := &Router{ds: ds, artwork: fa}
|
|
|
|
w, r := newImageRequest(dto.EncodeID(testID("deleted-item")))
|
|
api.getItemImage(w, r)
|
|
|
|
// 200 here is what separates a well-formed unknown id from a malformed one, which 404s.
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
Expect(fa.recvId).To(BeEmpty(), "an empty artwork id can only yield a placeholder")
|
|
})
|
|
|
|
It("sniffs the Content-Type instead of hardcoding it", func() {
|
|
ds := &tests.MockDataStore{}
|
|
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
|
|
|
|
png := append([]byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n'}, make([]byte, 512)...)
|
|
fa := &fakeArtwork{data: png}
|
|
api := &Router{ds: ds, artwork: fa}
|
|
|
|
w, r := newImageRequest(dto.EncodeID(testID("a1")))
|
|
api.getItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
Expect(w.Header().Get("Content-Type")).To(Equal("image/png"))
|
|
})
|
|
|
|
It("resolves a playlist's cover regardless of visibility, even for an anonymous caller", func() {
|
|
ds := &tests.MockDataStore{}
|
|
ds.Playlist(context.Background()).(*tests.MockPlaylistRepo).SetData(model.Playlists{{ID: testID("pl1"), Name: "Mix", OwnerID: testID("someone")}})
|
|
fa := &fakeArtwork{}
|
|
api := &Router{ds: ds, artwork: fa}
|
|
|
|
w, r := newImageRequest(dto.EncodeID(testID("pl1")))
|
|
api.getItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
Expect(fa.recvId).To(ContainSubstring(testID("pl1")))
|
|
})
|
|
|
|
// This endpoint is public (no user in the request), so artwork must be resolved under an
|
|
// elevated context; otherwise a private playlist's cover fails its visibility filter and
|
|
// silently falls back to the placeholder.
|
|
It("resolves artwork under an elevated admin context", func() {
|
|
ds := &tests.MockDataStore{}
|
|
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
|
|
fa := &fakeArtwork{}
|
|
api := &Router{ds: ds, artwork: fa}
|
|
|
|
w, r := newImageRequest(dto.EncodeID(testID("a1")))
|
|
api.getItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
u, ok := request.UserFrom(fa.recvCtx)
|
|
Expect(ok).To(BeTrue())
|
|
Expect(u.IsAdmin).To(BeTrue())
|
|
})
|
|
|
|
It("serves immutable when the tag param asserts the current hash", func() {
|
|
const hash = "0123456789abcdef"
|
|
ds := &tests.MockDataStore{}
|
|
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
|
|
fa := &fakeArtwork{hash: hash}
|
|
api := &Router{ds: ds, artwork: fa}
|
|
|
|
w, r := newImageRequest(dto.EncodeID(testID("a1")))
|
|
q := r.URL.Query()
|
|
q.Set("tag", hash)
|
|
r.URL.RawQuery = q.Encode()
|
|
api.getItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
Expect(w.Header().Get("Cache-Control")).To(Equal("public, max-age=31536000, immutable"))
|
|
Expect(w.Header().Get("ETag")).To(Equal(`"` + hash + `"`))
|
|
})
|
|
|
|
It("revalidates via no-cache when no tag is provided", func() {
|
|
const hash = "0123456789abcdef"
|
|
ds := &tests.MockDataStore{}
|
|
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
|
|
fa := &fakeArtwork{hash: hash}
|
|
api := &Router{ds: ds, artwork: fa}
|
|
|
|
w, r := newImageRequest(dto.EncodeID(testID("a1")))
|
|
api.getItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
Expect(w.Header().Get("Cache-Control")).To(Equal("public, no-cache"))
|
|
})
|
|
})
|
|
|
|
// Real image fixtures: postItemImage validates uploads by decoding them.
|
|
func pngBytes() []byte {
|
|
var b bytes.Buffer
|
|
Expect(png.Encode(&b, image.NewRGBA(image.Rect(0, 0, 1, 1)))).To(Succeed())
|
|
return b.Bytes()
|
|
}
|
|
|
|
func jpegBytes() []byte {
|
|
var b bytes.Buffer
|
|
Expect(jpeg.Encode(&b, image.NewRGBA(image.Rect(0, 0, 1, 1)), nil)).To(Succeed())
|
|
return b.Bytes()
|
|
}
|
|
|
|
func gifBytes() []byte {
|
|
var b bytes.Buffer
|
|
Expect(gif.Encode(&b, image.NewRGBA(image.Rect(0, 0, 1, 1)), nil)).To(Succeed())
|
|
return b.Bytes()
|
|
}
|
|
|
|
// 1x1 WebP (Go's webp support is decode-only, so this one is pre-encoded).
|
|
func webpBytes() []byte {
|
|
b, err := base64.StdEncoding.DecodeString(
|
|
"UklGRjwAAABXRUJQVlA4IDAAAADQAQCdASoBAAEAAgA0JaACdLoB+AADsAD+8Oj3/yC5YXXI1/8gP+QH/ID/+PIAAAA=")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
return b
|
|
}
|
|
|
|
var _ = Describe("postItemImage", func() {
|
|
var api *Router
|
|
var fp *fakePlaylists
|
|
|
|
BeforeEach(func() {
|
|
fp = &fakePlaylists{getByIDPls: &model.Playlist{ID: testID("pl1")}}
|
|
api = &Router{playlists: fp}
|
|
})
|
|
|
|
It("uploads a raw JPEG body and returns 204", func() {
|
|
body := jpegBytes()
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(body))
|
|
r.Header.Set("Content-Type", "image/jpeg")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNoContent))
|
|
Expect(fp.setImagePlaylistID).To(Equal(testID("pl1")))
|
|
Expect(fp.setImageBytes).To(Equal(body))
|
|
Expect(fp.setImageExt).To(Equal(".jpeg"))
|
|
})
|
|
|
|
It("base64-decodes the body and derives the extension from the actual format, not Content-Type", func() {
|
|
raw := pngBytes()
|
|
encoded := base64.StdEncoding.EncodeToString(raw)
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader([]byte(encoded)))
|
|
r.Header.Set("Content-Type", "image/jpeg") // lies: the payload is a PNG
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNoContent))
|
|
Expect(fp.setImageBytes).To(Equal(raw))
|
|
Expect(fp.setImageExt).To(Equal(".png"))
|
|
})
|
|
|
|
It("returns 501 for a non-playlist item, draining the body first", func() {
|
|
fp.getByIDPls = nil
|
|
fp.getByIDErr = model.ErrNotFound
|
|
bodyReader := bytes.NewReader([]byte("some-bytes-that-must-be-drained"))
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("al1"))+"/Images/Primary", bodyReader)
|
|
r.Header.Set("Content-Type", "image/jpeg")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("al1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNotImplemented))
|
|
Expect(bodyReader.Len()).To(Equal(0))
|
|
})
|
|
|
|
It("returns 500 when the service fails", func() {
|
|
fp.setImageErr = errors.New("boom")
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(jpegBytes()))
|
|
r.Header.Set("Content-Type", "image/jpeg")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusInternalServerError))
|
|
})
|
|
|
|
It("accepts a raw WebP body", func() {
|
|
body := webpBytes()
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(body))
|
|
r.Header.Set("Content-Type", "image/webp")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNoContent))
|
|
Expect(fp.setImageBytes).To(Equal(body))
|
|
Expect(fp.setImageExt).To(Equal(".webp"))
|
|
})
|
|
|
|
It("accepts a raw GIF body", func() {
|
|
body := gifBytes()
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(body))
|
|
r.Header.Set("Content-Type", "image/gif")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNoContent))
|
|
Expect(fp.setImageBytes).To(Equal(body))
|
|
Expect(fp.setImageExt).To(Equal(".gif"))
|
|
})
|
|
|
|
It("rejects an oversized body with 400, like the native endpoint", func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
conf.Server.MaxImageUploadSize = "16" // 16 bytes
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(jpegBytes()))
|
|
r.Header.Set("Content-Type", "image/jpeg")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusBadRequest))
|
|
Expect(fp.setImagePlaylistID).To(BeEmpty(), "must not persist an over-limit upload")
|
|
})
|
|
|
|
It("applies the size limit to the decoded image, not the base64 body", func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
img := pngBytes()
|
|
// The raw image is exactly at the limit; its base64 form is 4/3 bigger.
|
|
conf.Server.MaxImageUploadSize = strconv.Itoa(len(img))
|
|
body := []byte(base64.StdEncoding.EncodeToString(img))
|
|
Expect(len(body)).To(BeNumerically(">", len(img)))
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(body))
|
|
r.Header.Set("Content-Type", "image/png")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNoContent))
|
|
Expect(fp.setImageBytes).To(Equal(img))
|
|
})
|
|
|
|
It("rejects a base64 body whose decoded image exceeds the limit with 400", func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
img := pngBytes()
|
|
conf.Server.MaxImageUploadSize = strconv.Itoa(len(img) - 1)
|
|
body := []byte(base64.StdEncoding.EncodeToString(img))
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(body))
|
|
r.Header.Set("Content-Type", "image/png")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusBadRequest))
|
|
Expect(fp.setImagePlaylistID).To(BeEmpty())
|
|
})
|
|
|
|
It("rejects a body that is neither an image nor base64 with 400", func() {
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", strings.NewReader("!!not base64!!"))
|
|
r.Header.Set("Content-Type", "image/jpeg")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusBadRequest))
|
|
Expect(fp.setImagePlaylistID).To(BeEmpty())
|
|
})
|
|
|
|
It("rejects bytes that sniff as an image but don't decode (e.g. a truncated or renamed file)", func() {
|
|
body := []byte{0xFF, 0xD8, 0xFF, 0xE0, 0x00, 0x10, 'J', 'F', 'I', 'F'} // JPEG magic, not a JPEG
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(body))
|
|
r.Header.Set("Content-Type", "image/jpeg")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusBadRequest))
|
|
Expect(fp.setImagePlaylistID).To(BeEmpty())
|
|
})
|
|
|
|
It("forbids a non-admin upload when artwork upload is disabled", func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
conf.Server.EnableArtworkUpload = false
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(jpegBytes()))
|
|
r.Header.Set("Content-Type", "image/jpeg")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
r = r.WithContext(request.WithUser(r.Context(), model.User{ID: testID("u1"), IsAdmin: false}))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusForbidden))
|
|
Expect(fp.setImagePlaylistID).To(BeEmpty())
|
|
})
|
|
|
|
It("still allows an admin upload when artwork upload is disabled", func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
conf.Server.EnableArtworkUpload = false
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", bytes.NewReader(jpegBytes()))
|
|
r.Header.Set("Content-Type", "image/jpeg")
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
r = r.WithContext(request.WithUser(r.Context(), model.User{ID: testID("admin"), IsAdmin: true}))
|
|
|
|
api.postItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNoContent))
|
|
})
|
|
})
|
|
|
|
var _ = Describe("deleteItemImage", func() {
|
|
It("removes the playlist image and returns 204", func() {
|
|
fp := &fakePlaylists{getByIDPls: &model.Playlist{ID: testID("pl1")}}
|
|
api := &Router{playlists: fp}
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("DELETE", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", nil)
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.deleteItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNoContent))
|
|
Expect(fp.removeImagePlaylistID).To(Equal(testID("pl1")))
|
|
})
|
|
|
|
It("returns 501 for a non-playlist item", func() {
|
|
fp := &fakePlaylists{getByIDErr: model.ErrNotFound}
|
|
api := &Router{playlists: fp}
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("DELETE", "/Items/"+dto.EncodeID(testID("al1"))+"/Images/Primary", nil)
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("al1")))
|
|
|
|
api.deleteItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusNotImplemented))
|
|
})
|
|
|
|
It("returns 500 when the service fails", func() {
|
|
fp := &fakePlaylists{getByIDPls: &model.Playlist{ID: testID("pl1")}, removeImageErr: errors.New("boom")}
|
|
api := &Router{playlists: fp}
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("DELETE", "/Items/"+dto.EncodeID(testID("pl1"))+"/Images/Primary", nil)
|
|
r = withChiURLParam(r, "itemId", dto.EncodeID(testID("pl1")))
|
|
|
|
api.deleteItemImage(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusInternalServerError))
|
|
})
|
|
})
|