From b0cb40b02940c660525f8c739a9b26757d2bb39a Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 17 Feb 2026 09:00:15 -0500 Subject: [PATCH 1/9] feat: add custom NATURALSORT collation for natural number ordering Register a custom SQLite collation function (NATURALSORT) that compares strings using natural sort ordering, where embedded numeric sequences are compared as numbers rather than lexicographically. This fixes the issue where albums like "Bravo Hits 1-132" sort as 1, 10, 100... instead of 1, 2, 3... 10, 11... Closes navidrome/navidrome#4891 --- db/db.go | 6 +- ...000_add_natural_sort_collation_indexes.sql | 118 ++++++++++++++ persistence/collation_test.go | 24 +-- persistence/helpers.go | 14 +- persistence/helpers_test.go | 16 +- persistence/sql_base_repository.go | 13 +- utils/str/natural_sort.go | 142 +++++++++++++++++ utils/str/natural_sort_test.go | 150 ++++++++++++++++++ 8 files changed, 460 insertions(+), 23 deletions(-) create mode 100644 db/migrations/20260216200000_add_natural_sort_collation_indexes.sql create mode 100644 utils/str/natural_sort.go create mode 100644 utils/str/natural_sort_test.go diff --git a/db/db.go b/db/db.go index 0945d1a00..e69661697 100644 --- a/db/db.go +++ b/db/db.go @@ -13,6 +13,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/utils/hasher" "github.com/navidrome/navidrome/utils/singleton" + "github.com/navidrome/navidrome/utils/str" "github.com/pressly/goose/v3" ) @@ -31,7 +32,10 @@ func Db() *sql.DB { return singleton.GetInstance(func() *sql.DB { sql.Register(Driver, &sqlite3.SQLiteDriver{ ConnectHook: func(conn *sqlite3.SQLiteConn) error { - return conn.RegisterFunc("SEEDEDRAND", hasher.HashFunc(), false) + if err := conn.RegisterFunc("SEEDEDRAND", hasher.HashFunc(), false); err != nil { + return err + } + return conn.RegisterCollation("NATURALSORT", str.NaturalSortCompare) }, }) Path = conf.Server.DbPath diff --git a/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql b/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql new file mode 100644 index 000000000..24bcaf405 --- /dev/null +++ b/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql @@ -0,0 +1,118 @@ +-- +goose Up + +-- Recreate indexes on order_* and sort expression fields to use NATURALSORT collation. +-- This enables natural number ordering (e.g., "Album 2" before "Album 10"). + +-- Artist indexes +drop index if exists artist_order_artist_name; +create index artist_order_artist_name + on artist (order_artist_name collate NATURALSORT); + +drop index if exists artist_sort_name; +create index artist_sort_name + on artist (coalesce(nullif(sort_artist_name,''),order_artist_name) collate NATURALSORT); + +-- Album indexes +drop index if exists album_order_album_name; +create index album_order_album_name + on album (order_album_name collate NATURALSORT); + +drop index if exists album_order_album_artist_name; +create index album_order_album_artist_name + on album (order_album_artist_name collate NATURALSORT); + +drop index if exists album_alphabetical_by_artist; +create index album_alphabetical_by_artist + on album (compilation, order_album_artist_name collate NATURALSORT, order_album_name collate NATURALSORT); + +drop index if exists album_sort_name; +create index album_sort_name + on album (coalesce(nullif(sort_album_name,''),order_album_name) collate NATURALSORT); + +drop index if exists album_sort_album_artist_name; +create index album_sort_album_artist_name + on album (coalesce(nullif(sort_album_artist_name,''),order_album_artist_name) collate NATURALSORT); + +-- Media file indexes +drop index if exists media_file_order_title; +create index media_file_order_title + on media_file (order_title collate NATURALSORT); + +drop index if exists media_file_order_album_name; +create index media_file_order_album_name + on media_file (order_album_name collate NATURALSORT); + +drop index if exists media_file_order_artist_name; +create index media_file_order_artist_name + on media_file (order_artist_name collate NATURALSORT); + +drop index if exists media_file_sort_title; +create index media_file_sort_title + on media_file (coalesce(nullif(sort_title,''),order_title) collate NATURALSORT); + +drop index if exists media_file_sort_artist_name; +create index media_file_sort_artist_name + on media_file (coalesce(nullif(sort_artist_name,''),order_artist_name) collate NATURALSORT); + +drop index if exists media_file_sort_album_name; +create index media_file_sort_album_name + on media_file (coalesce(nullif(sort_album_name,''),order_album_name) collate NATURALSORT); + +-- +goose Down + +-- Restore NOCASE collation indexes + +-- Artist indexes +drop index if exists artist_order_artist_name; +create index artist_order_artist_name + on artist (order_artist_name); + +drop index if exists artist_sort_name; +create index artist_sort_name + on artist (coalesce(nullif(sort_artist_name,''),order_artist_name) collate NOCASE); + +-- Album indexes +drop index if exists album_order_album_name; +create index album_order_album_name + on album (order_album_name); + +drop index if exists album_order_album_artist_name; +create index album_order_album_artist_name + on album (order_album_artist_name); + +drop index if exists album_alphabetical_by_artist; +create index album_alphabetical_by_artist + on album (compilation, order_album_artist_name, order_album_name); + +drop index if exists album_sort_name; +create index album_sort_name + on album (coalesce(nullif(sort_album_name,''),order_album_name) collate NOCASE); + +drop index if exists album_sort_album_artist_name; +create index album_sort_album_artist_name + on album (coalesce(nullif(sort_album_artist_name,''),order_album_artist_name) collate NOCASE); + +-- Media file indexes +drop index if exists media_file_order_title; +create index media_file_order_title + on media_file (order_title); + +drop index if exists media_file_order_album_name; +create index media_file_order_album_name + on media_file (order_album_name); + +drop index if exists media_file_order_artist_name; +create index media_file_order_artist_name + on media_file (order_artist_name); + +drop index if exists media_file_sort_title; +create index media_file_sort_title + on media_file (coalesce(nullif(sort_title,''),order_title) collate NOCASE); + +drop index if exists media_file_sort_artist_name; +create index media_file_sort_artist_name + on media_file (coalesce(nullif(sort_artist_name,''),order_artist_name) collate NOCASE); + +drop index if exists media_file_sort_album_name; +create index media_file_sort_album_name + on media_file (coalesce(nullif(sort_album_name,''),order_album_name) collate NOCASE); diff --git a/persistence/collation_test.go b/persistence/collation_test.go index bb1276577..24fb5ed13 100644 --- a/persistence/collation_test.go +++ b/persistence/collation_test.go @@ -41,18 +41,18 @@ var _ = Describe("Collation", func() { func(table, column string) { Expect(checkIndexUsage(conn, table, column)).To(Succeed()) }, - Entry("artist.order_artist_name", "artist", "order_artist_name collate nocase"), - Entry("artist.sort_artist_name", "artist", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate nocase"), - Entry("album.order_album_name", "album", "order_album_name collate nocase"), - Entry("album.order_album_artist_name", "album", "order_album_artist_name collate nocase"), - Entry("album.sort_album_name", "album", "coalesce(nullif(sort_album_name,''),order_album_name) collate nocase"), - Entry("album.sort_album_artist_name", "album", "coalesce(nullif(sort_album_artist_name,''),order_album_artist_name) collate nocase"), - Entry("media_file.order_title", "media_file", "order_title collate nocase"), - Entry("media_file.order_album_name", "media_file", "order_album_name collate nocase"), - Entry("media_file.order_artist_name", "media_file", "order_artist_name collate nocase"), - Entry("media_file.sort_title", "media_file", "coalesce(nullif(sort_title,''),order_title) collate nocase"), - Entry("media_file.sort_album_name", "media_file", "coalesce(nullif(sort_album_name,''),order_album_name) collate nocase"), - Entry("media_file.sort_artist_name", "media_file", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate nocase"), + Entry("artist.order_artist_name", "artist", "order_artist_name collate NATURALSORT"), + Entry("artist.sort_artist_name", "artist", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate NATURALSORT"), + Entry("album.order_album_name", "album", "order_album_name collate NATURALSORT"), + Entry("album.order_album_artist_name", "album", "order_album_artist_name collate NATURALSORT"), + Entry("album.sort_album_name", "album", "coalesce(nullif(sort_album_name,''),order_album_name) collate NATURALSORT"), + Entry("album.sort_album_artist_name", "album", "coalesce(nullif(sort_album_artist_name,''),order_album_artist_name) collate NATURALSORT"), + Entry("media_file.order_title", "media_file", "order_title collate NATURALSORT"), + Entry("media_file.order_album_name", "media_file", "order_album_name collate NATURALSORT"), + Entry("media_file.order_artist_name", "media_file", "order_artist_name collate NATURALSORT"), + Entry("media_file.sort_title", "media_file", "coalesce(nullif(sort_title,''),order_title) collate NATURALSORT"), + Entry("media_file.sort_album_name", "media_file", "coalesce(nullif(sort_album_name,''),order_album_name) collate NATURALSORT"), + Entry("media_file.sort_artist_name", "media_file", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate NATURALSORT"), Entry("media_file.path", "media_file", "path collate nocase"), Entry("playlist.name", "playlist", "name collate nocase"), Entry("radio.name", "radio", "name collate nocase"), diff --git a/persistence/helpers.go b/persistence/helpers.go index fd6a9a4cd..f96dc127b 100644 --- a/persistence/helpers.go +++ b/persistence/helpers.go @@ -82,11 +82,19 @@ func (e existsCond) ToSql() (string, []any, error) { var sortOrderRegex = regexp.MustCompile(`order_([a-z_]+)`) -// Convert the order_* columns to an expression using sort_* columns. Example: -// sort_album_name -> (coalesce(nullif(sort_album_name,”),order_album_name) collate nocase) +// mapSortOrder converts order_* columns to an expression using sort_* columns with NATURALSORT collation. Example: +// order_album_name -> (coalesce(nullif(sort_album_name,”),order_album_name) collate NATURALSORT) // It finds order column names anywhere in the substring func mapSortOrder(tableName, order string) string { order = strings.ToLower(order) - repl := fmt.Sprintf("(coalesce(nullif(%[1]s.sort_$1,''),%[1]s.order_$1) collate nocase)", tableName) + repl := fmt.Sprintf("(coalesce(nullif(%[1]s.sort_$1,''),%[1]s.order_$1) collate NATURALSORT)", tableName) return sortOrderRegex.ReplaceAllString(order, repl) } + +// mapNaturalSortCollation wraps bare order_* column references with NATURALSORT collation. Example: +// order_album_name -> (order_album_name collate NATURALSORT) +// It finds order column names anywhere in the substring +func mapNaturalSortCollation(order string) string { + order = strings.ToLower(order) + return sortOrderRegex.ReplaceAllString(order, "(order_$1 collate NATURALSORT)") +} diff --git a/persistence/helpers_test.go b/persistence/helpers_test.go index 85893ef55..a1bd30651 100644 --- a/persistence/helpers_test.go +++ b/persistence/helpers_test.go @@ -94,13 +94,23 @@ var _ = Describe("Helpers", func() { sort := "ORDER_ALBUM_NAME asc" mapped := mapSortOrder("album", sort) Expect(mapped).To(Equal(`(coalesce(nullif(album.sort_album_name,''),album.order_album_name)` + - ` collate nocase) asc`)) + ` collate NATURALSORT) asc`)) }) It("changes multiple order columns to sort expressions", func() { sort := "compilation, order_title asc, order_album_artist_name desc, year desc" mapped := mapSortOrder("album", sort) - Expect(mapped).To(Equal(`compilation, (coalesce(nullif(album.sort_title,''),album.order_title) collate nocase) asc,` + - ` (coalesce(nullif(album.sort_album_artist_name,''),album.order_album_artist_name) collate nocase) desc, year desc`)) + Expect(mapped).To(Equal(`compilation, (coalesce(nullif(album.sort_title,''),album.order_title) collate NATURALSORT) asc,` + + ` (coalesce(nullif(album.sort_album_artist_name,''),album.order_album_artist_name) collate NATURALSORT) desc, year desc`)) + }) + It("wraps bare order columns with NATURALSORT collation", func() { + sort := "order_album_name, order_album_artist_name" + mapped := mapNaturalSortCollation(sort) + Expect(mapped).To(Equal(`(order_album_name collate NATURALSORT), (order_album_artist_name collate NATURALSORT)`)) + }) + It("does not change non-order columns in mapNaturalSortCollation", func() { + sort := "compilation, year desc" + mapped := mapNaturalSortCollation(sort) + Expect(mapped).To(Equal(sort)) }) }) }) diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go index fd263d37b..e5ef0e6f6 100644 --- a/persistence/sql_base_repository.go +++ b/persistence/sql_base_repository.go @@ -71,9 +71,12 @@ func (r *sqlRepository) registerModel(instance any, filters map[string]filterFun // // If PreferSortTags is enabled, it will map the order fields to the corresponding sort expression, // which gives precedence to sort tags. -// Ex: order_title => (coalesce(nullif(sort_title,”),order_title) collate nocase) +// Ex: order_title => (coalesce(nullif(sort_title,"),order_title) collate NATURALSORT) // To avoid performance issues, indexes should be created for these sort expressions // +// All order_* fields are wrapped with NATURALSORT collation to enable natural number ordering +// (e.g., "Album 2" sorts before "Album 10"). +// // NOTE: if an individual item has spaces, it should be wrapped in parentheses. For example, // you should write "(lyrics != '[]')". This prevents the item being split unexpectedly. // Without parentheses, "lyrics != '[]'" would be mapped as simply "lyrics" @@ -82,11 +85,13 @@ func (r *sqlRepository) setSortMappings(mappings map[string]string, tableName .. if len(tableName) > 0 { tn = tableName[0] } - if conf.Server.PreferSortTags { - for k, v := range mappings { + for k, v := range mappings { + if conf.Server.PreferSortTags { v = mapSortOrder(tn, v) - mappings[k] = v + } else { + v = mapNaturalSortCollation(v) } + mappings[k] = v } r.sortMappings = mappings } diff --git a/utils/str/natural_sort.go b/utils/str/natural_sort.go new file mode 100644 index 000000000..0a52bfc57 --- /dev/null +++ b/utils/str/natural_sort.go @@ -0,0 +1,142 @@ +package str + +import ( + "strings" +) + +// NaturalSortCompare compares two strings using natural sort ordering, +// where embedded numeric sequences are compared as numbers rather than +// lexicographically. For example, "track2" < "track10" (unlike lexicographic +// ordering which gives "track10" < "track2"). The comparison is also +// case-insensitive. +func NaturalSortCompare(a, b string) int { + ia, ib := 0, 0 + for ia < len(a) && ib < len(b) { + ca := a[ia] + cb := b[ib] + + // If both characters are digits, compare the full numeric sequences + if isDigit(ca) && isDigit(cb) { + result := compareNumericChunks(a, b, &ia, &ib) + if result != 0 { + return result + } + continue + } + + // Case-insensitive character comparison + la := toLower(ca) + lb := toLower(cb) + if la != lb { + if la < lb { + return -1 + } + return 1 + } + + ia++ + ib++ + } + + // The shorter string comes first if all else is equal + return len(a) - len(b) +} + +// compareNumericChunks compares two numeric sequences starting at positions +// ia and ib in strings a and b. It advances the position indices past the +// numeric sequences. Numbers are compared by value, with leading zeros +// used as a tiebreaker (fewer leading zeros comes first). +func compareNumericChunks(a, b string, ia, ib *int) int { + // Skip leading zeros and count them + zerosA := 0 + for *ia < len(a) && a[*ia] == '0' { + zerosA++ + *ia++ + } + zerosB := 0 + for *ib < len(b) && b[*ib] == '0' { + zerosB++ + *ib++ + } + + // Find the extent of the remaining digits + startA := *ia + for *ia < len(a) && isDigit(a[*ia]) { + *ia++ + } + startB := *ib + for *ib < len(b) && isDigit(b[*ib]) { + *ib++ + } + + lenA := *ia - startA + lenB := *ib - startB + + // More significant digits means a larger number + if lenA != lenB { + return lenA - lenB + } + + // Same number of significant digits - compare digit by digit + for i := 0; i < lenA; i++ { + if a[startA+i] != b[startB+i] { + if a[startA+i] < b[startB+i] { + return -1 + } + return 1 + } + } + + // Same numeric value - fewer leading zeros comes first + if zerosA != zerosB { + return zerosA - zerosB + } + + return 0 +} + +func isDigit(c byte) bool { + return c >= '0' && c <= '9' +} + +func toLower(c byte) byte { + if c >= 'A' && c <= 'Z' { + return c + ('a' - 'A') + } + return c +} + +// NaturalSortKey transforms a string into a key that, when compared +// lexicographically with NOCASE collation, produces natural sort order. +// Numeric sequences are zero-padded to a fixed width so that lexicographic +// comparison yields numeric ordering. +func NaturalSortKey(s string) string { + const padWidth = 20 // Enough for uint64 max + + var b strings.Builder + b.Grow(len(s) + padWidth) // Pre-allocate a reasonable size + + i := 0 + for i < len(s) { + if isDigit(s[i]) { + // Find the full numeric sequence + start := i + for i < len(s) && isDigit(s[i]) { + i++ + } + numStr := s[start:i] + + // Pad the number with leading zeros to padWidth + if len(numStr) < padWidth { + for j := 0; j < padWidth-len(numStr); j++ { + b.WriteByte('0') + } + } + b.WriteString(numStr) + } else { + b.WriteByte(s[i]) + i++ + } + } + return b.String() +} diff --git a/utils/str/natural_sort_test.go b/utils/str/natural_sort_test.go new file mode 100644 index 000000000..fab0d7232 --- /dev/null +++ b/utils/str/natural_sort_test.go @@ -0,0 +1,150 @@ +package str_test + +import ( + "testing" + + "github.com/navidrome/navidrome/utils/str" +) + +func TestNaturalSortCompare(t *testing.T) { + tests := []struct { + name string + a, b string + want int // -1, 0, or 1 + }{ + // Basic string comparison + {"equal strings", "abc", "abc", 0}, + {"less than", "abc", "abd", -1}, + {"greater than", "abd", "abc", 1}, + + // Case insensitive + {"case insensitive equal", "ABC", "abc", 0}, + {"case insensitive less", "ABC", "abd", -1}, + {"case insensitive mixed", "aBc", "AbC", 0}, + + // Numeric ordering + {"single digit order", "track1", "track2", -1}, + {"natural sort 2 vs 10", "track2", "track10", -1}, + {"natural sort 9 vs 10", "track9", "track10", -1}, + {"natural sort 10 vs 2", "track10", "track2", 1}, + {"natural sort 1 vs 100", "track1", "track100", -1}, + + // Album numbering (main use case from issue) + {"album numbers 1 vs 2", "Bravo Hits 1", "Bravo Hits 2", -1}, + {"album numbers 2 vs 10", "Bravo Hits 2", "Bravo Hits 10", -1}, + {"album numbers 10 vs 100", "Bravo Hits 10", "Bravo Hits 100", -1}, + {"album numbers 9 vs 10", "Bravo Hits 9", "Bravo Hits 10", -1}, + {"album numbers equal", "Bravo Hits 10", "Bravo Hits 10", 0}, + {"album numbers 99 vs 100", "Bravo Hits 99", "Bravo Hits 100", -1}, + {"album numbers 100 vs 101", "Bravo Hits 100", "Bravo Hits 101", -1}, + + // Pure numeric strings + {"pure number 1 vs 2", "1", "2", -1}, + {"pure number 2 vs 10", "2", "10", -1}, + {"pure number 10 vs 9", "10", "9", 1}, + {"pure number equal", "42", "42", 0}, + + // Leading zeros (same numeric value, but more leading zeros sorts later) + {"leading zeros same value", "01", "1", 1}, + {"leading zeros 02 vs 1", "02", "1", 1}, + {"leading zeros 01 vs 10", "01", "10", -1}, + + // Multiple numeric sequences + {"multi number 1.2 vs 1.10", "file1.2", "file1.10", -1}, + {"multi number 2.1 vs 10.1", "file2.1", "file10.1", -1}, + + // Empty strings + {"both empty", "", "", 0}, + {"empty vs non-empty", "", "a", -1}, + {"non-empty vs empty", "a", "", 1}, + + // Strings with only numbers at different positions + {"number prefix vs alpha prefix", "1abc", "abc", -1}, + {"alpha prefix vs number prefix", "abc", "1abc", 1}, + + // Edge cases + {"same prefix different length", "abc", "abcd", -1}, + {"numbers at beginning", "10abc", "9abc", 1}, + {"numbers at end", "abc10", "abc9", 1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := str.NaturalSortCompare(tt.a, tt.b) + // Normalize to -1, 0, 1 for comparison + gotNorm := normalize(got) + if gotNorm != tt.want { + t.Errorf("NaturalSortCompare(%q, %q) = %d (normalized: %d), want %d", tt.a, tt.b, got, gotNorm, tt.want) + } + }) + } +} + +func TestNaturalSortCompare_Symmetry(t *testing.T) { + pairs := [][2]string{ + {"track2", "track10"}, + {"abc", "abd"}, + {"Bravo Hits 9", "Bravo Hits 10"}, + {"1", "2"}, + {"file1.2", "file1.10"}, + } + + for _, pair := range pairs { + a, b := pair[0], pair[1] + ab := str.NaturalSortCompare(a, b) + ba := str.NaturalSortCompare(b, a) + if normalize(ab) != -normalize(ba) { + t.Errorf("Symmetry violated: Compare(%q,%q)=%d but Compare(%q,%q)=%d", a, b, ab, b, a, ba) + } + } +} + +func TestNaturalSortKey(t *testing.T) { + // Test that sorting by NaturalSortKey produces natural order + inputs := []string{ + "Bravo Hits 1", + "Bravo Hits 10", + "Bravo Hits 100", + "Bravo Hits 2", + "Bravo Hits 20", + "Bravo Hits 3", + "Bravo Hits 9", + } + expected := []string{ + "Bravo Hits 1", + "Bravo Hits 2", + "Bravo Hits 3", + "Bravo Hits 9", + "Bravo Hits 10", + "Bravo Hits 20", + "Bravo Hits 100", + } + + // Get the keys and verify they sort correctly + keys := make([]string, len(inputs)) + for i, s := range inputs { + keys[i] = str.NaturalSortKey(s) + } + + // Verify the expected order produces keys in ascending order + expectedKeys := make([]string, len(expected)) + for i, s := range expected { + expectedKeys[i] = str.NaturalSortKey(s) + } + for i := 1; i < len(expectedKeys); i++ { + if expectedKeys[i] <= expectedKeys[i-1] { + t.Errorf("NaturalSortKey order violation: key(%q) = %q should be > key(%q) = %q", + expected[i], expectedKeys[i], expected[i-1], expectedKeys[i-1]) + } + } +} + +func normalize(v int) int { + if v < 0 { + return -1 + } + if v > 0 { + return 1 + } + return 0 +} From 9bcefea0ca405044416156903f76250abcddaf30 Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 17 Feb 2026 09:00:20 -0500 Subject: [PATCH 2/9] refactor: use maruel/natural for NATURALSORT collation instead of custom impl Replace the hand-rolled natural sort comparison with a thin wrapper around github.com/maruel/natural.Compare, which is already a dependency. The wrapper just lowercases both inputs for case-insensitive comparison. --- utils/str/natural_sort.go | 132 +-------------------------------- utils/str/natural_sort_test.go | 49 ------------ 2 files changed, 3 insertions(+), 178 deletions(-) diff --git a/utils/str/natural_sort.go b/utils/str/natural_sort.go index 0a52bfc57..281114e1f 100644 --- a/utils/str/natural_sort.go +++ b/utils/str/natural_sort.go @@ -2,6 +2,8 @@ package str import ( "strings" + + "github.com/maruel/natural" ) // NaturalSortCompare compares two strings using natural sort ordering, @@ -10,133 +12,5 @@ import ( // ordering which gives "track10" < "track2"). The comparison is also // case-insensitive. func NaturalSortCompare(a, b string) int { - ia, ib := 0, 0 - for ia < len(a) && ib < len(b) { - ca := a[ia] - cb := b[ib] - - // If both characters are digits, compare the full numeric sequences - if isDigit(ca) && isDigit(cb) { - result := compareNumericChunks(a, b, &ia, &ib) - if result != 0 { - return result - } - continue - } - - // Case-insensitive character comparison - la := toLower(ca) - lb := toLower(cb) - if la != lb { - if la < lb { - return -1 - } - return 1 - } - - ia++ - ib++ - } - - // The shorter string comes first if all else is equal - return len(a) - len(b) -} - -// compareNumericChunks compares two numeric sequences starting at positions -// ia and ib in strings a and b. It advances the position indices past the -// numeric sequences. Numbers are compared by value, with leading zeros -// used as a tiebreaker (fewer leading zeros comes first). -func compareNumericChunks(a, b string, ia, ib *int) int { - // Skip leading zeros and count them - zerosA := 0 - for *ia < len(a) && a[*ia] == '0' { - zerosA++ - *ia++ - } - zerosB := 0 - for *ib < len(b) && b[*ib] == '0' { - zerosB++ - *ib++ - } - - // Find the extent of the remaining digits - startA := *ia - for *ia < len(a) && isDigit(a[*ia]) { - *ia++ - } - startB := *ib - for *ib < len(b) && isDigit(b[*ib]) { - *ib++ - } - - lenA := *ia - startA - lenB := *ib - startB - - // More significant digits means a larger number - if lenA != lenB { - return lenA - lenB - } - - // Same number of significant digits - compare digit by digit - for i := 0; i < lenA; i++ { - if a[startA+i] != b[startB+i] { - if a[startA+i] < b[startB+i] { - return -1 - } - return 1 - } - } - - // Same numeric value - fewer leading zeros comes first - if zerosA != zerosB { - return zerosA - zerosB - } - - return 0 -} - -func isDigit(c byte) bool { - return c >= '0' && c <= '9' -} - -func toLower(c byte) byte { - if c >= 'A' && c <= 'Z' { - return c + ('a' - 'A') - } - return c -} - -// NaturalSortKey transforms a string into a key that, when compared -// lexicographically with NOCASE collation, produces natural sort order. -// Numeric sequences are zero-padded to a fixed width so that lexicographic -// comparison yields numeric ordering. -func NaturalSortKey(s string) string { - const padWidth = 20 // Enough for uint64 max - - var b strings.Builder - b.Grow(len(s) + padWidth) // Pre-allocate a reasonable size - - i := 0 - for i < len(s) { - if isDigit(s[i]) { - // Find the full numeric sequence - start := i - for i < len(s) && isDigit(s[i]) { - i++ - } - numStr := s[start:i] - - // Pad the number with leading zeros to padWidth - if len(numStr) < padWidth { - for j := 0; j < padWidth-len(numStr); j++ { - b.WriteByte('0') - } - } - b.WriteString(numStr) - } else { - b.WriteByte(s[i]) - i++ - } - } - return b.String() + return natural.Compare(strings.ToLower(a), strings.ToLower(b)) } diff --git a/utils/str/natural_sort_test.go b/utils/str/natural_sort_test.go index fab0d7232..514c79b69 100644 --- a/utils/str/natural_sort_test.go +++ b/utils/str/natural_sort_test.go @@ -44,11 +44,6 @@ func TestNaturalSortCompare(t *testing.T) { {"pure number 10 vs 9", "10", "9", 1}, {"pure number equal", "42", "42", 0}, - // Leading zeros (same numeric value, but more leading zeros sorts later) - {"leading zeros same value", "01", "1", 1}, - {"leading zeros 02 vs 1", "02", "1", 1}, - {"leading zeros 01 vs 10", "01", "10", -1}, - // Multiple numeric sequences {"multi number 1.2 vs 1.10", "file1.2", "file1.10", -1}, {"multi number 2.1 vs 10.1", "file2.1", "file10.1", -1}, @@ -58,10 +53,6 @@ func TestNaturalSortCompare(t *testing.T) { {"empty vs non-empty", "", "a", -1}, {"non-empty vs empty", "a", "", 1}, - // Strings with only numbers at different positions - {"number prefix vs alpha prefix", "1abc", "abc", -1}, - {"alpha prefix vs number prefix", "abc", "1abc", 1}, - // Edge cases {"same prefix different length", "abc", "abcd", -1}, {"numbers at beginning", "10abc", "9abc", 1}, @@ -99,46 +90,6 @@ func TestNaturalSortCompare_Symmetry(t *testing.T) { } } -func TestNaturalSortKey(t *testing.T) { - // Test that sorting by NaturalSortKey produces natural order - inputs := []string{ - "Bravo Hits 1", - "Bravo Hits 10", - "Bravo Hits 100", - "Bravo Hits 2", - "Bravo Hits 20", - "Bravo Hits 3", - "Bravo Hits 9", - } - expected := []string{ - "Bravo Hits 1", - "Bravo Hits 2", - "Bravo Hits 3", - "Bravo Hits 9", - "Bravo Hits 10", - "Bravo Hits 20", - "Bravo Hits 100", - } - - // Get the keys and verify they sort correctly - keys := make([]string, len(inputs)) - for i, s := range inputs { - keys[i] = str.NaturalSortKey(s) - } - - // Verify the expected order produces keys in ascending order - expectedKeys := make([]string, len(expected)) - for i, s := range expected { - expectedKeys[i] = str.NaturalSortKey(s) - } - for i := 1; i < len(expectedKeys); i++ { - if expectedKeys[i] <= expectedKeys[i-1] { - t.Errorf("NaturalSortKey order violation: key(%q) = %q should be > key(%q) = %q", - expected[i], expectedKeys[i], expected[i-1], expectedKeys[i-1]) - } - } -} - func normalize(v int) int { if v < 0 { return -1 From 929e7193b4c11f01cab6aa7527431f6b44234fe3 Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 17 Feb 2026 09:00:23 -0500 Subject: [PATCH 3/9] refactor: use natural.Compare directly instead of wrapper --- db/db.go | 4 +- utils/str/natural_sort.go | 16 ------ utils/str/natural_sort_test.go | 101 --------------------------------- 3 files changed, 2 insertions(+), 119 deletions(-) delete mode 100644 utils/str/natural_sort.go delete mode 100644 utils/str/natural_sort_test.go diff --git a/db/db.go b/db/db.go index e69661697..ff852c740 100644 --- a/db/db.go +++ b/db/db.go @@ -7,13 +7,13 @@ import ( "fmt" "runtime" + "github.com/maruel/natural" "github.com/mattn/go-sqlite3" "github.com/navidrome/navidrome/conf" _ "github.com/navidrome/navidrome/db/migrations" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/utils/hasher" "github.com/navidrome/navidrome/utils/singleton" - "github.com/navidrome/navidrome/utils/str" "github.com/pressly/goose/v3" ) @@ -35,7 +35,7 @@ func Db() *sql.DB { if err := conn.RegisterFunc("SEEDEDRAND", hasher.HashFunc(), false); err != nil { return err } - return conn.RegisterCollation("NATURALSORT", str.NaturalSortCompare) + return conn.RegisterCollation("NATURALSORT", natural.Compare) }, }) Path = conf.Server.DbPath diff --git a/utils/str/natural_sort.go b/utils/str/natural_sort.go deleted file mode 100644 index 281114e1f..000000000 --- a/utils/str/natural_sort.go +++ /dev/null @@ -1,16 +0,0 @@ -package str - -import ( - "strings" - - "github.com/maruel/natural" -) - -// NaturalSortCompare compares two strings using natural sort ordering, -// where embedded numeric sequences are compared as numbers rather than -// lexicographically. For example, "track2" < "track10" (unlike lexicographic -// ordering which gives "track10" < "track2"). The comparison is also -// case-insensitive. -func NaturalSortCompare(a, b string) int { - return natural.Compare(strings.ToLower(a), strings.ToLower(b)) -} diff --git a/utils/str/natural_sort_test.go b/utils/str/natural_sort_test.go deleted file mode 100644 index 514c79b69..000000000 --- a/utils/str/natural_sort_test.go +++ /dev/null @@ -1,101 +0,0 @@ -package str_test - -import ( - "testing" - - "github.com/navidrome/navidrome/utils/str" -) - -func TestNaturalSortCompare(t *testing.T) { - tests := []struct { - name string - a, b string - want int // -1, 0, or 1 - }{ - // Basic string comparison - {"equal strings", "abc", "abc", 0}, - {"less than", "abc", "abd", -1}, - {"greater than", "abd", "abc", 1}, - - // Case insensitive - {"case insensitive equal", "ABC", "abc", 0}, - {"case insensitive less", "ABC", "abd", -1}, - {"case insensitive mixed", "aBc", "AbC", 0}, - - // Numeric ordering - {"single digit order", "track1", "track2", -1}, - {"natural sort 2 vs 10", "track2", "track10", -1}, - {"natural sort 9 vs 10", "track9", "track10", -1}, - {"natural sort 10 vs 2", "track10", "track2", 1}, - {"natural sort 1 vs 100", "track1", "track100", -1}, - - // Album numbering (main use case from issue) - {"album numbers 1 vs 2", "Bravo Hits 1", "Bravo Hits 2", -1}, - {"album numbers 2 vs 10", "Bravo Hits 2", "Bravo Hits 10", -1}, - {"album numbers 10 vs 100", "Bravo Hits 10", "Bravo Hits 100", -1}, - {"album numbers 9 vs 10", "Bravo Hits 9", "Bravo Hits 10", -1}, - {"album numbers equal", "Bravo Hits 10", "Bravo Hits 10", 0}, - {"album numbers 99 vs 100", "Bravo Hits 99", "Bravo Hits 100", -1}, - {"album numbers 100 vs 101", "Bravo Hits 100", "Bravo Hits 101", -1}, - - // Pure numeric strings - {"pure number 1 vs 2", "1", "2", -1}, - {"pure number 2 vs 10", "2", "10", -1}, - {"pure number 10 vs 9", "10", "9", 1}, - {"pure number equal", "42", "42", 0}, - - // Multiple numeric sequences - {"multi number 1.2 vs 1.10", "file1.2", "file1.10", -1}, - {"multi number 2.1 vs 10.1", "file2.1", "file10.1", -1}, - - // Empty strings - {"both empty", "", "", 0}, - {"empty vs non-empty", "", "a", -1}, - {"non-empty vs empty", "a", "", 1}, - - // Edge cases - {"same prefix different length", "abc", "abcd", -1}, - {"numbers at beginning", "10abc", "9abc", 1}, - {"numbers at end", "abc10", "abc9", 1}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := str.NaturalSortCompare(tt.a, tt.b) - // Normalize to -1, 0, 1 for comparison - gotNorm := normalize(got) - if gotNorm != tt.want { - t.Errorf("NaturalSortCompare(%q, %q) = %d (normalized: %d), want %d", tt.a, tt.b, got, gotNorm, tt.want) - } - }) - } -} - -func TestNaturalSortCompare_Symmetry(t *testing.T) { - pairs := [][2]string{ - {"track2", "track10"}, - {"abc", "abd"}, - {"Bravo Hits 9", "Bravo Hits 10"}, - {"1", "2"}, - {"file1.2", "file1.10"}, - } - - for _, pair := range pairs { - a, b := pair[0], pair[1] - ab := str.NaturalSortCompare(a, b) - ba := str.NaturalSortCompare(b, a) - if normalize(ab) != -normalize(ba) { - t.Errorf("Symmetry violated: Compare(%q,%q)=%d but Compare(%q,%q)=%d", a, b, ab, b, a, ba) - } - } -} - -func normalize(v int) int { - if v < 0 { - return -1 - } - if v > 0 { - return 1 - } - return 0 -} From 86c326bd4a91d4a9b891e6f6aa96e0faeae64b3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Tue, 17 Feb 2026 09:30:18 -0500 Subject: [PATCH 4/9] fix doc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- persistence/sql_base_repository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go index e5ef0e6f6..231d0905e 100644 --- a/persistence/sql_base_repository.go +++ b/persistence/sql_base_repository.go @@ -71,7 +71,7 @@ func (r *sqlRepository) registerModel(instance any, filters map[string]filterFun // // If PreferSortTags is enabled, it will map the order fields to the corresponding sort expression, // which gives precedence to sort tags. -// Ex: order_title => (coalesce(nullif(sort_title,"),order_title) collate NATURALSORT) +// Ex: order_title => (coalesce(nullif(sort_title,''), order_title) collate NATURALSORT) // To avoid performance issues, indexes should be created for these sort expressions // // All order_* fields are wrapped with NATURALSORT collation to enable natural number ordering From 8e647a0e413c116f844cbefddfc6530a2c173a6c Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 17 Feb 2026 09:26:45 -0500 Subject: [PATCH 5/9] chore(deps): bump golangci-lint to v2.10.0 and suppress new gosec false positives Bump golangci-lint from v2.9.0 to v2.10.0, which includes a newer gosec with additional taint-analysis rules (G117, G703, G704, G705) and a stricter G101 check. Added inline //nolint:gosec comments to suppress 21 false positives across 19 files: struct fields flagged as secrets (G117), w.Write calls flagged as XSS (G705), HTTP client calls flagged as SSRF (G704), os.Stat/os.ReadFile/os.Remove flagged as path traversal (G703), and a sort mapping flagged as hardcoded credentials (G101). Signed-off-by: Deluan --- Makefile | 2 +- adapters/deezer/client_auth.go | 2 +- adapters/lastfm/auth_router.go | 2 +- adapters/listenbrainz/client.go | 2 +- conf/configuration.go | 10 +++++----- core/artwork/sources.go | 2 +- core/metrics/insights.go | 2 +- core/storage/local/local.go | 2 +- model/user.go | 2 +- persistence/artist_repository.go | 2 +- scanner/external.go | 2 +- server/backgrounds/handler.go | 2 +- server/events/sse.go | 2 +- server/nativeapi/inspect.go | 2 +- server/nativeapi/native_api.go | 4 ++-- server/nativeapi/playlists.go | 10 +++++----- server/nativeapi/queue.go | 2 +- server/public/handle_shares.go | 2 +- server/server.go | 2 +- server/subsonic/api.go | 2 +- 20 files changed, 29 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index 52aa5a8cd..dea912b9b 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ DOCKER_TAG ?= deluan/navidrome:develop # Taglib version to use in cross-compilation, from https://github.com/navidrome/cross-taglib CROSS_TAGLIB_VERSION ?= 2.1.1-2 -GOLANGCI_LINT_VERSION ?= v2.9.0 +GOLANGCI_LINT_VERSION ?= v2.10.0 UI_SRC_FILES := $(shell find ui -type f -not -path "ui/build/*" -not -path "ui/node_modules/*") diff --git a/adapters/deezer/client_auth.go b/adapters/deezer/client_auth.go index c88c2bcb6..d0924b768 100644 --- a/adapters/deezer/client_auth.go +++ b/adapters/deezer/client_auth.go @@ -65,7 +65,7 @@ func (c *client) getJWT(ctx context.Context) (string, error) { } type authResponse struct { - JWT string `json:"jwt"` + JWT string `json:"jwt"` //nolint:gosec } var result authResponse diff --git a/adapters/lastfm/auth_router.go b/adapters/lastfm/auth_router.go index 0052f73d7..162ae9037 100644 --- a/adapters/lastfm/auth_router.go +++ b/adapters/lastfm/auth_router.go @@ -110,7 +110,7 @@ func (s *Router) callback(w http.ResponseWriter, r *http.Request) { if err != nil { w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte("An error occurred while authorizing with Last.fm. \n\nRequest ID: " + middleware.GetReqID(ctx))) + _, _ = w.Write([]byte("An error occurred while authorizing with Last.fm. \n\nRequest ID: " + middleware.GetReqID(ctx))) //nolint:gosec return } diff --git a/adapters/listenbrainz/client.go b/adapters/listenbrainz/client.go index 0427ed07f..708f02f28 100644 --- a/adapters/listenbrainz/client.go +++ b/adapters/listenbrainz/client.go @@ -57,7 +57,7 @@ type listenBrainzResponse struct { } type listenBrainzRequest struct { - ApiKey string + ApiKey string //nolint:gosec Body listenBrainzRequestBody } diff --git a/conf/configuration.go b/conf/configuration.go index e2aca6c13..8994c1c88 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -172,8 +172,8 @@ type TagConf struct { type lastfmOptions struct { Enabled bool - ApiKey string - Secret string + ApiKey string //nolint:gosec + Secret string //nolint:gosec Language string ScrobbleFirstArtistOnly bool @@ -183,7 +183,7 @@ type lastfmOptions struct { type spotifyOptions struct { ID string - Secret string + Secret string //nolint:gosec } type deezerOptions struct { @@ -208,7 +208,7 @@ type httpHeaderOptions struct { type prometheusOptions struct { Enabled bool MetricsPath string - Password string + Password string //nolint:gosec } type AudioDeviceDefinition []string @@ -748,7 +748,7 @@ func getConfigFile(cfgFile string) string { } cfgFile = os.Getenv("ND_CONFIGFILE") if cfgFile != "" { - if _, err := os.Stat(cfgFile); err == nil { + if _, err := os.Stat(cfgFile); err == nil { //nolint:gosec return cfgFile } } diff --git a/core/artwork/sources.go b/core/artwork/sources.go index c7da7b19b..b1b9b5454 100644 --- a/core/artwork/sources.go +++ b/core/artwork/sources.go @@ -230,7 +230,7 @@ func fromURL(ctx context.Context, imageUrl *url.URL) (io.ReadCloser, string, err hc := http.Client{Timeout: 5 * time.Second} req, _ := http.NewRequestWithContext(ctx, http.MethodGet, imageUrl.String(), nil) req.Header.Set("User-Agent", consts.HTTPUserAgent) - resp, err := hc.Do(req) + resp, err := hc.Do(req) //nolint:gosec if err != nil { return nil, "", err } diff --git a/core/metrics/insights.go b/core/metrics/insights.go index 07162dedb..849ddd6fc 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -108,7 +108,7 @@ func (c *insightsCollector) sendInsights(ctx context.Context) { return } req.Header.Set("Content-Type", "application/json") - resp, err := hc.Do(req) + resp, err := hc.Do(req) //nolint:gosec if err != nil { log.Trace(ctx, "Could not send Insights data", err) return diff --git a/core/storage/local/local.go b/core/storage/local/local.go index 5c335ddb9..cd60c9ef1 100644 --- a/core/storage/local/local.go +++ b/core/storage/local/local.go @@ -44,7 +44,7 @@ func newLocalStorage(u url.URL) storage.Storage { func (s *localStorage) FS() (storage.MusicFS, error) { path := s.u.Path - if _, err := os.Stat(path); err != nil { + if _, err := os.Stat(path); err != nil { //nolint:gosec return nil, fmt.Errorf("%w: %s", err, path) } return &localFS{FS: os.DirFS(path), extractor: s.extractor}, nil diff --git a/model/user.go b/model/user.go index 2127b635c..1c8541ccf 100644 --- a/model/user.go +++ b/model/user.go @@ -22,7 +22,7 @@ type User struct { Password string `structs:"-" json:"-"` // This is used to set or change a password when calling Put. If it is empty, the password is not changed. // It is received from the UI with the name "password" - NewPassword string `structs:"password,omitempty" json:"password,omitempty"` + NewPassword string `structs:"password,omitempty" json:"password,omitempty"` //nolint:gosec // If changing the password, this is also required CurrentPassword string `structs:"current_password,omitempty" json:"currentPassword,omitempty"` } diff --git a/persistence/artist_repository.go b/persistence/artist_repository.go index b888256a4..5623bd7f8 100644 --- a/persistence/artist_repository.go +++ b/persistence/artist_repository.go @@ -138,7 +138,7 @@ func NewArtistRepository(ctx context.Context, db dbx.Builder) model.ArtistReposi "missing": booleanFilter, "library_id": artistLibraryIdFilter, }) - r.setSortMappings(map[string]string{ + r.setSortMappings(map[string]string{ //nolint:gosec "name": "order_artist_name", "starred_at": "starred, starred_at", "rated_at": "rating, rated_at", diff --git a/scanner/external.go b/scanner/external.go index 75ee2bead..29ca90be6 100644 --- a/scanner/external.go +++ b/scanner/external.go @@ -158,7 +158,7 @@ func writeTargetsToFile(targets []model.ScanTarget) (string, error) { for _, target := range targets { if _, err := fmt.Fprintln(tmpFile, target.String()); err != nil { - os.Remove(tmpFile.Name()) + os.Remove(tmpFile.Name()) //nolint:gosec return "", fmt.Errorf("failed to write to temp file: %w", err) } } diff --git a/server/backgrounds/handler.go b/server/backgrounds/handler.go index 61b7d48b8..b00a51696 100644 --- a/server/backgrounds/handler.go +++ b/server/backgrounds/handler.go @@ -80,7 +80,7 @@ func (h *Handler) serveImage(ctx context.Context, item cache.Item) (io.Reader, e } c := http.Client{Timeout: imageRequestTimeout} req, _ := http.NewRequestWithContext(ctx, http.MethodGet, imageURL(image), nil) - resp, err := c.Do(req) //nolint:bodyclose // No need to close resp.Body, it will be closed via the CachedStream wrapper + resp, err := c.Do(req) //nolint:bodyclose,gosec // No need to close resp.Body, it will be closed via the CachedStream wrapper if errors.Is(err, context.DeadlineExceeded) { defaultImage, _ := base64.StdEncoding.DecodeString(consts.DefaultUILoginBackgroundOffline) return strings.NewReader(string(defaultImage)), nil diff --git a/server/events/sse.go b/server/events/sse.go index 54a602985..39e217160 100644 --- a/server/events/sse.go +++ b/server/events/sse.go @@ -104,7 +104,7 @@ func writeEvent(ctx context.Context, w io.Writer, event message, timeout time.Du log.Debug(ctx, "Error setting write timeout", err) } - _, err := fmt.Fprintf(w, "id: %d\nevent: %s\ndata: %s\n\n", event.id, event.event, event.data) + _, err := fmt.Fprintf(w, "id: %d\nevent: %s\ndata: %s\n\n", event.id, event.event, event.data) //nolint:gosec if err != nil { return err } diff --git a/server/nativeapi/inspect.go b/server/nativeapi/inspect.go index 3178395ce..7c96312ed 100644 --- a/server/nativeapi/inspect.go +++ b/server/nativeapi/inspect.go @@ -60,7 +60,7 @@ func inspect(ds model.DataStore) http.HandlerFunc { w.Header().Set("Content-Type", "application/json") - if _, err := w.Write(response); err != nil { + if _, err := w.Write(response); err != nil { //nolint:gosec log.Error(ctx, "Error sending response to client", err) } } diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index 91ddd0fa3..52e633bee 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -207,7 +207,7 @@ func writeDeleteManyResponse(w http.ResponseWriter, r *http.Request, ids []strin http.Error(w, err.Error(), http.StatusInternalServerError) } } - _, err = w.Write(resp) + _, err = w.Write(resp) //nolint:gosec if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) } @@ -243,7 +243,7 @@ func (api *Router) addInsightsRoute(r chi.Router) { r.Get("/insights/*", func(w http.ResponseWriter, r *http.Request) { last, success := api.insights.LastRun(r.Context()) if conf.Server.EnableInsightsCollector { - _, _ = w.Write([]byte(`{"id":"insights_status", "lastRun":"` + last.Format("2006-01-02 15:04:05") + `", "success":` + strconv.FormatBool(success) + `}`)) + _, _ = w.Write([]byte(`{"id":"insights_status", "lastRun":"` + last.Format("2006-01-02 15:04:05") + `", "success":` + strconv.FormatBool(success) + `}`)) //nolint:gosec } else { _, _ = w.Write([]byte(`{"id":"insights_status", "lastRun":"disabled", "success":false}`)) } diff --git a/server/nativeapi/playlists.go b/server/nativeapi/playlists.go index afa964e15..1e2c5e07e 100644 --- a/server/nativeapi/playlists.go +++ b/server/nativeapi/playlists.go @@ -59,7 +59,7 @@ func createPlaylistFromM3U(playlists core.Playlists) http.HandlerFunc { return } w.WriteHeader(http.StatusCreated) - _, err = w.Write([]byte(pls.ToM3U8())) + _, err = w.Write([]byte(pls.ToM3U8())) //nolint:gosec if err != nil { log.Error(ctx, "Error sending m3u contents", err) http.Error(w, err.Error(), http.StatusInternalServerError) @@ -90,7 +90,7 @@ func handleExportPlaylist(ds model.DataStore) http.HandlerFunc { disposition := fmt.Sprintf("attachment; filename=\"%s.m3u\"", pls.Name) w.Header().Set("Content-Disposition", disposition) - _, err = w.Write([]byte(pls.ToM3U8())) + _, err = w.Write([]byte(pls.ToM3U8())) //nolint:gosec if err != nil { log.Error(ctx, "Error sending playlist", "name", pls.Name) return @@ -162,7 +162,7 @@ func addToPlaylist(ds model.DataStore) http.HandlerFunc { count += c // Must return an object with an ID, to satisfy ReactAdmin `create` call - _, err = fmt.Fprintf(w, `{"added":%d}`, count) + _, err = fmt.Fprintf(w, `{"added":%d}`, count) //nolint:gosec if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) } @@ -204,7 +204,7 @@ func reorderItem(ds model.DataStore) http.HandlerFunc { return } - _, err = w.Write(fmt.Appendf(nil, `{"id":"%d"}`, id)) + _, err = w.Write(fmt.Appendf(nil, `{"id":"%d"}`, id)) //nolint:gosec if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) } @@ -225,6 +225,6 @@ func getSongPlaylists(ds model.DataStore) http.HandlerFunc { http.Error(w, err.Error(), http.StatusInternalServerError) return } - _, _ = w.Write(data) + _, _ = w.Write(data) //nolint:gosec } } diff --git a/server/nativeapi/queue.go b/server/nativeapi/queue.go index 0a3136660..a7700c02c 100644 --- a/server/nativeapi/queue.go +++ b/server/nativeapi/queue.go @@ -87,7 +87,7 @@ func getQueue(ds model.DataStore) http.HandlerFunc { return } w.Header().Set("Content-Type", "application/json") - _, _ = w.Write(resp) + _, _ = w.Write(resp) //nolint:gosec } } diff --git a/server/public/handle_shares.go b/server/public/handle_shares.go index ad8a5da6b..36764dece 100644 --- a/server/public/handle_shares.go +++ b/server/public/handle_shares.go @@ -59,7 +59,7 @@ func (pub *Router) handleM3U(w http.ResponseWriter, r *http.Request) { s = pub.mapShareToM3U(r, *s) w.WriteHeader(http.StatusOK) w.Header().Set("Content-Type", "audio/x-mpegurl") - _, _ = w.Write([]byte(s.ToM3U8())) + _, _ = w.Write([]byte(s.ToM3U8())) //nolint:gosec } func checkShareError(ctx context.Context, w http.ResponseWriter, err error, id string) { diff --git a/server/server.go b/server/server.go index aa9043ba4..b05c20cc5 100644 --- a/server/server.go +++ b/server/server.go @@ -244,7 +244,7 @@ func (s *Server) frontendAssetsHandler() http.Handler { // It provides detailed error messages for common issues like encrypted private keys. func validateTLSCertificates(certFile, keyFile string) error { // Read the key file to check for encryption - keyData, err := os.ReadFile(keyFile) + keyData, err := os.ReadFile(keyFile) //nolint:gosec if err != nil { return fmt.Errorf("reading TLS key file: %w", err) } diff --git a/server/subsonic/api.go b/server/subsonic/api.go index 1d13e2c0a..c3108ea5b 100644 --- a/server/subsonic/api.go +++ b/server/subsonic/api.go @@ -363,7 +363,7 @@ func sendResponse(w http.ResponseWriter, r *http.Request, payload *responses.Sub } } - if _, err := w.Write(response); err != nil { + if _, err := w.Write(response); err != nil { //nolint:gosec log.Error(r, "Error sending response to client", "endpoint", r.URL.Path, "payload", string(response), err) } } From 24ab04581a5a9aa391e7145b35a4263474c4c09d Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 17 Feb 2026 09:45:34 -0500 Subject: [PATCH 6/9] fix format Signed-off-by: Deluan --- persistence/sql_base_repository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go index 231d0905e..f9db7b9a7 100644 --- a/persistence/sql_base_repository.go +++ b/persistence/sql_base_repository.go @@ -71,7 +71,7 @@ func (r *sqlRepository) registerModel(instance any, filters map[string]filterFun // // If PreferSortTags is enabled, it will map the order fields to the corresponding sort expression, // which gives precedence to sort tags. -// Ex: order_title => (coalesce(nullif(sort_title,''), order_title) collate NATURALSORT) +// Ex: order_title => (coalesce(nullif(sort_title,""), order_title) collate NATURALSORT) // To avoid performance issues, indexes should be created for these sort expressions // // All order_* fields are wrapped with NATURALSORT collation to enable natural number ordering From 90d6cd5f47590199033c731aff853e05c9a3c415 Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 17 Feb 2026 10:37:08 -0500 Subject: [PATCH 7/9] refactor: update collation handling for natural sorting in SQL queries Signed-off-by: Deluan --- db/db.go | 5 ++- ...000_add_natural_sort_collation_indexes.sql | 16 ++++++++ persistence/collation_test.go | 40 +++++++++---------- persistence/helpers.go | 8 ---- persistence/helpers_test.go | 10 ----- persistence/sql_base_repository.go | 14 +++---- 6 files changed, 46 insertions(+), 47 deletions(-) diff --git a/db/db.go b/db/db.go index ff852c740..ac795cd5c 100644 --- a/db/db.go +++ b/db/db.go @@ -6,6 +6,7 @@ import ( "embed" "fmt" "runtime" + "strings" "github.com/maruel/natural" "github.com/mattn/go-sqlite3" @@ -35,7 +36,9 @@ func Db() *sql.DB { if err := conn.RegisterFunc("SEEDEDRAND", hasher.HashFunc(), false); err != nil { return err } - return conn.RegisterCollation("NATURALSORT", natural.Compare) + return conn.RegisterCollation("NATURALSORT", func(a, b string) int { + return natural.Compare(strings.ToLower(a), strings.ToLower(b)) + }) }, }) Path = conf.Server.DbPath diff --git a/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql b/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql index 24bcaf405..d9eea1b8b 100644 --- a/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql +++ b/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql @@ -1,5 +1,14 @@ -- +goose Up +-- Change order_*/sort_* column collation from NOCASE to NATURALSORT. +-- This way bare ORDER BY on these columns automatically uses natural sorting, +-- without needing explicit COLLATE NATURALSORT in every query. +PRAGMA writable_schema = ON; +UPDATE sqlite_master +SET sql = replace(sql, 'collate NOCASE', 'collate NATURALSORT') +WHERE type = 'table' AND name IN ('artist', 'album', 'media_file'); +PRAGMA writable_schema = OFF; + -- Recreate indexes on order_* and sort expression fields to use NATURALSORT collation. -- This enables natural number ordering (e.g., "Album 2" before "Album 10"). @@ -60,6 +69,13 @@ create index media_file_sort_album_name -- +goose Down +-- Restore NOCASE column collation +PRAGMA writable_schema = ON; +UPDATE sqlite_master +SET sql = replace(sql, 'collate NATURALSORT', 'collate NOCASE') +WHERE type = 'table' AND name IN ('artist', 'album', 'media_file'); +PRAGMA writable_schema = OFF; + -- Restore NOCASE collation indexes -- Artist indexes diff --git a/persistence/collation_test.go b/persistence/collation_test.go index 24fb5ed13..ceaccfb26 100644 --- a/persistence/collation_test.go +++ b/persistence/collation_test.go @@ -17,24 +17,24 @@ import ( var _ = Describe("Collation", func() { conn := db.Db() DescribeTable("Column collation", - func(table, column string) { - Expect(checkCollation(conn, table, column)).To(Succeed()) + func(table, column, expectedCollation string) { + Expect(checkCollation(conn, table, column, expectedCollation)).To(Succeed()) }, - Entry("artist.order_artist_name", "artist", "order_artist_name"), - Entry("artist.sort_artist_name", "artist", "sort_artist_name"), - Entry("album.order_album_name", "album", "order_album_name"), - Entry("album.order_album_artist_name", "album", "order_album_artist_name"), - Entry("album.sort_album_name", "album", "sort_album_name"), - Entry("album.sort_album_artist_name", "album", "sort_album_artist_name"), - Entry("media_file.order_title", "media_file", "order_title"), - Entry("media_file.order_album_name", "media_file", "order_album_name"), - Entry("media_file.order_artist_name", "media_file", "order_artist_name"), - Entry("media_file.sort_title", "media_file", "sort_title"), - Entry("media_file.sort_album_name", "media_file", "sort_album_name"), - Entry("media_file.sort_artist_name", "media_file", "sort_artist_name"), - Entry("playlist.name", "playlist", "name"), - Entry("radio.name", "radio", "name"), - Entry("user.name", "user", "name"), + Entry("artist.order_artist_name", "artist", "order_artist_name", "NATURALSORT"), + Entry("artist.sort_artist_name", "artist", "sort_artist_name", "NATURALSORT"), + Entry("album.order_album_name", "album", "order_album_name", "NATURALSORT"), + Entry("album.order_album_artist_name", "album", "order_album_artist_name", "NATURALSORT"), + Entry("album.sort_album_name", "album", "sort_album_name", "NATURALSORT"), + Entry("album.sort_album_artist_name", "album", "sort_album_artist_name", "NATURALSORT"), + Entry("media_file.order_title", "media_file", "order_title", "NATURALSORT"), + Entry("media_file.order_album_name", "media_file", "order_album_name", "NATURALSORT"), + Entry("media_file.order_artist_name", "media_file", "order_artist_name", "NATURALSORT"), + Entry("media_file.sort_title", "media_file", "sort_title", "NATURALSORT"), + Entry("media_file.sort_album_name", "media_file", "sort_album_name", "NATURALSORT"), + Entry("media_file.sort_artist_name", "media_file", "sort_artist_name", "NATURALSORT"), + Entry("playlist.name", "playlist", "name", "NOCASE"), + Entry("radio.name", "radio", "name", "NOCASE"), + Entry("user.name", "user", "name", "NOCASE"), ) DescribeTable("Index collation", @@ -91,7 +91,7 @@ order by %[2]s`, table, column)) return errors.New("no rows returned") } -func checkCollation(conn *sql.DB, table string, column string) error { +func checkCollation(conn *sql.DB, table, column, expectedCollation string) error { rows, err := conn.Query(fmt.Sprintf("SELECT sql FROM sqlite_master WHERE type='table' AND tbl_name='%s'", table)) if err != nil { return err @@ -113,12 +113,12 @@ func checkCollation(conn *sql.DB, table string, column string) error { if !re.MatchString(res) { return fmt.Errorf("column '%s' not found in table '%s'", column, table) } - re = regexp.MustCompile(fmt.Sprintf(`(?i)\b%s\b.*collate\s+NOCASE`, column)) + re = regexp.MustCompile(fmt.Sprintf(`(?i)\b%s\b.*collate\s+%s`, column, expectedCollation)) if re.MatchString(res) { return nil } } else { return fmt.Errorf("table '%s' not found", table) } - return fmt.Errorf("column '%s' in table '%s' does not have NOCASE collation", column, table) + return fmt.Errorf("column '%s' in table '%s' does not have %s collation", column, table, expectedCollation) } diff --git a/persistence/helpers.go b/persistence/helpers.go index f96dc127b..3f58b3480 100644 --- a/persistence/helpers.go +++ b/persistence/helpers.go @@ -90,11 +90,3 @@ func mapSortOrder(tableName, order string) string { repl := fmt.Sprintf("(coalesce(nullif(%[1]s.sort_$1,''),%[1]s.order_$1) collate NATURALSORT)", tableName) return sortOrderRegex.ReplaceAllString(order, repl) } - -// mapNaturalSortCollation wraps bare order_* column references with NATURALSORT collation. Example: -// order_album_name -> (order_album_name collate NATURALSORT) -// It finds order column names anywhere in the substring -func mapNaturalSortCollation(order string) string { - order = strings.ToLower(order) - return sortOrderRegex.ReplaceAllString(order, "(order_$1 collate NATURALSORT)") -} diff --git a/persistence/helpers_test.go b/persistence/helpers_test.go index a1bd30651..dfb7c3f21 100644 --- a/persistence/helpers_test.go +++ b/persistence/helpers_test.go @@ -102,15 +102,5 @@ var _ = Describe("Helpers", func() { Expect(mapped).To(Equal(`compilation, (coalesce(nullif(album.sort_title,''),album.order_title) collate NATURALSORT) asc,` + ` (coalesce(nullif(album.sort_album_artist_name,''),album.order_album_artist_name) collate NATURALSORT) desc, year desc`)) }) - It("wraps bare order columns with NATURALSORT collation", func() { - sort := "order_album_name, order_album_artist_name" - mapped := mapNaturalSortCollation(sort) - Expect(mapped).To(Equal(`(order_album_name collate NATURALSORT), (order_album_artist_name collate NATURALSORT)`)) - }) - It("does not change non-order columns in mapNaturalSortCollation", func() { - sort := "compilation, year desc" - mapped := mapNaturalSortCollation(sort) - Expect(mapped).To(Equal(sort)) - }) }) }) diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go index f9db7b9a7..73de71744 100644 --- a/persistence/sql_base_repository.go +++ b/persistence/sql_base_repository.go @@ -72,10 +72,10 @@ func (r *sqlRepository) registerModel(instance any, filters map[string]filterFun // If PreferSortTags is enabled, it will map the order fields to the corresponding sort expression, // which gives precedence to sort tags. // Ex: order_title => (coalesce(nullif(sort_title,""), order_title) collate NATURALSORT) -// To avoid performance issues, indexes should be created for these sort expressions +// To avoid performance issues, indexes should be created for these sort expressions. // -// All order_* fields are wrapped with NATURALSORT collation to enable natural number ordering -// (e.g., "Album 2" sorts before "Album 10"). +// When PreferSortTags is off, bare order_* columns automatically use their column-defined NATURALSORT +// collation, so no query-time wrapping is needed. // // NOTE: if an individual item has spaces, it should be wrapped in parentheses. For example, // you should write "(lyrics != '[]')". This prevents the item being split unexpectedly. @@ -85,13 +85,11 @@ func (r *sqlRepository) setSortMappings(mappings map[string]string, tableName .. if len(tableName) > 0 { tn = tableName[0] } - for k, v := range mappings { - if conf.Server.PreferSortTags { + if conf.Server.PreferSortTags { + for k, v := range mappings { v = mapSortOrder(tn, v) - } else { - v = mapNaturalSortCollation(v) + mappings[k] = v } - mappings[k] = v } r.sortMappings = mappings } From e766a5d78085060573d86ada40b004341d1e295e Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 17 Feb 2026 10:42:05 -0500 Subject: [PATCH 8/9] refactor: extend NATURALSORT collation to playlist and radio indexes Signed-off-by: Deluan --- ...000_add_natural_sort_collation_indexes.sql | 22 +++++++++++++++++-- persistence/collation_test.go | 8 +++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql b/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql index d9eea1b8b..a92234ba6 100644 --- a/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql +++ b/db/migrations/20260216200000_add_natural_sort_collation_indexes.sql @@ -6,7 +6,7 @@ PRAGMA writable_schema = ON; UPDATE sqlite_master SET sql = replace(sql, 'collate NOCASE', 'collate NATURALSORT') -WHERE type = 'table' AND name IN ('artist', 'album', 'media_file'); +WHERE type = 'table' AND name IN ('artist', 'album', 'media_file', 'playlist', 'radio'); PRAGMA writable_schema = OFF; -- Recreate indexes on order_* and sort expression fields to use NATURALSORT collation. @@ -67,13 +67,22 @@ drop index if exists media_file_sort_album_name; create index media_file_sort_album_name on media_file (coalesce(nullif(sort_album_name,''),order_album_name) collate NATURALSORT); +-- Playlist and radio indexes: recreate to match new NATURALSORT column collation +drop index if exists playlist_name; +create index playlist_name + on playlist (name collate NATURALSORT); + +drop index if exists radio_name; +create index radio_name + on radio (name collate NATURALSORT); + -- +goose Down -- Restore NOCASE column collation PRAGMA writable_schema = ON; UPDATE sqlite_master SET sql = replace(sql, 'collate NATURALSORT', 'collate NOCASE') -WHERE type = 'table' AND name IN ('artist', 'album', 'media_file'); +WHERE type = 'table' AND name IN ('artist', 'album', 'media_file', 'playlist', 'radio'); PRAGMA writable_schema = OFF; -- Restore NOCASE collation indexes @@ -132,3 +141,12 @@ create index media_file_sort_artist_name drop index if exists media_file_sort_album_name; create index media_file_sort_album_name on media_file (coalesce(nullif(sort_album_name,''),order_album_name) collate NOCASE); + +-- Restore playlist and radio indexes +drop index if exists playlist_name; +create index playlist_name + on playlist (name); + +drop index if exists radio_name; +create index radio_name + on radio (name); diff --git a/persistence/collation_test.go b/persistence/collation_test.go index ceaccfb26..61a28c106 100644 --- a/persistence/collation_test.go +++ b/persistence/collation_test.go @@ -32,8 +32,8 @@ var _ = Describe("Collation", func() { Entry("media_file.sort_title", "media_file", "sort_title", "NATURALSORT"), Entry("media_file.sort_album_name", "media_file", "sort_album_name", "NATURALSORT"), Entry("media_file.sort_artist_name", "media_file", "sort_artist_name", "NATURALSORT"), - Entry("playlist.name", "playlist", "name", "NOCASE"), - Entry("radio.name", "radio", "name", "NOCASE"), + Entry("playlist.name", "playlist", "name", "NATURALSORT"), + Entry("radio.name", "radio", "name", "NATURALSORT"), Entry("user.name", "user", "name", "NOCASE"), ) @@ -54,8 +54,8 @@ var _ = Describe("Collation", func() { Entry("media_file.sort_album_name", "media_file", "coalesce(nullif(sort_album_name,''),order_album_name) collate NATURALSORT"), Entry("media_file.sort_artist_name", "media_file", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate NATURALSORT"), Entry("media_file.path", "media_file", "path collate nocase"), - Entry("playlist.name", "playlist", "name collate nocase"), - Entry("radio.name", "radio", "name collate nocase"), + Entry("playlist.name", "playlist", "name collate NATURALSORT"), + Entry("radio.name", "radio", "name collate NATURALSORT"), Entry("user.user_name", "user", "user_name collate nocase"), ) }) From b4b183051391baf1e3227dc85e9973ef4f1c885a Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 17 Feb 2026 11:35:36 -0500 Subject: [PATCH 9/9] refactor: remove outdated comments regarding NATURALSORT collation behavior Signed-off-by: Deluan --- persistence/sql_base_repository.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go index 73de71744..64968670c 100644 --- a/persistence/sql_base_repository.go +++ b/persistence/sql_base_repository.go @@ -74,9 +74,6 @@ func (r *sqlRepository) registerModel(instance any, filters map[string]filterFun // Ex: order_title => (coalesce(nullif(sort_title,""), order_title) collate NATURALSORT) // To avoid performance issues, indexes should be created for these sort expressions. // -// When PreferSortTags is off, bare order_* columns automatically use their column-defined NATURALSORT -// collation, so no query-time wrapping is needed. -// // NOTE: if an individual item has spaces, it should be wrapped in parentheses. For example, // you should write "(lyrics != '[]')". This prevents the item being split unexpectedly. // Without parentheses, "lyrics != '[]'" would be mapped as simply "lyrics"