fix(playlists): match REST cols case-insensitively (PR #5542 review)

Go's encoding/json populates struct fields from case-variant keys like
{"Name":"x"} or {"OwnerId":"y"}, but rest.Put's getFieldNames extracts
raw JSON keys verbatim. With case-sensitive matching, sentFields would
ignore the field on the update side — a request with {"Name":"Renamed"}
would parse into entity.Name but then sent("name") returns false and
the rename silently no-ops.

Normalize both sides to lowercase. The entity-based owner-permission
guard added in the previous commit remains as belt-and-suspenders but
is now redundant with this change.

Also clarify the applyContentUpdate doc comment: namePtr/commentPtr
are nil when the field is absent OR present-but-unchanged, while
publicPtr only tracks presence (an idempotent public is still forwarded).
This commit is contained in:
Deluan 2026-05-27 23:21:24 -03:00
parent a75f820fa3
commit 3060eef63f
2 changed files with 23 additions and 6 deletions

View File

@ -4,6 +4,7 @@ import (
"context"
"errors"
"reflect"
"strings"
"github.com/deluan/rest"
"github.com/navidrome/navidrome/model"
@ -126,8 +127,10 @@ func (s *playlists) updatePlaylistEntity(ctx context.Context, id string, entity
// applyContentUpdate handles updates that change at least one of name/comment/
// owner/rules. It goes through updateMetadata, which always bumps updatedAt
// (invalidating cached cover-art URLs). Pointer args are nil for fields not
// present in the request.
// (invalidating cached cover-art URLs). namePtr/commentPtr are nil when the
// field is absent from the request OR present-but-unchanged (so updateMetadata
// skips them); publicPtr is nil only when public is absent from the request
// (an idempotent public value is still forwarded).
func (s *playlists) applyContentUpdate(ctx context.Context, current, entity *model.Playlist,
sent func(string) bool, nameChanged, commentChanged, ownerChanged, rulesChanged bool,
) error {
@ -175,15 +178,17 @@ func (s *playlists) applyFlagsOnly(ctx context.Context, current, entity *model.P
}
// sentFields returns a predicate that reports whether a JSON field was present
// in the request body. An empty cols list means "treat the entity as a full
// record" — every field is considered sent.
// in the request body. Matching is case-insensitive to mirror Go's json
// decoder, which populates struct fields from case-variant keys like
// {"Name":"x"} or {"OWNERID":"y"}. An empty cols list means "treat the entity
// as a full record" — every field is considered sent.
func sentFields(cols []string) func(string) bool {
if len(cols) == 0 {
return func(string) bool { return true }
}
set := slice.ToMap(cols, func(c string) (string, struct{}) { return c, struct{}{} })
set := slice.ToMap(cols, func(c string) (string, struct{}) { return strings.ToLower(c), struct{}{} })
return func(field string) bool {
_, ok := set[field]
_, ok := set[strings.ToLower(field)]
return ok
}
}

View File

@ -367,6 +367,18 @@ var _ = Describe("REST Adapter", func() {
err := repo.Update("partial", &model.Playlist{Public: true}, "public")
Expect(err).ToNot(HaveOccurred())
})
It("matches cols case-insensitively (mirrors json decoder behavior)", func() {
// Go's json decoder populates struct fields from case-variant keys
// like {"Name":"x"}, but rest.Put's field-name extraction is
// case-sensitive. sentFields normalizes both sides so a request
// with {"Name":"Renamed"} is honored, not silently ignored.
repo = ps.NewRepository(ctx).(rest.Persistable)
err := repo.Update("partial", &model.Playlist{Name: "Renamed"}, "Name")
Expect(err).ToNot(HaveOccurred())
Expect(mockPlsRepo.Last.Name).To(Equal("Renamed"))
Expect(mockPlsRepo.Last.Comment).To(Equal("Original comment"))
})
})
})