diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 232171c6d..0767346fa 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -217,7 +217,7 @@ jobs: CROSS_TAGLIB_VERSION=${{ env.CROSS_TAGLIB_VERSION }} - name: Upload Binaries - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v5 with: name: navidrome-${{ env.PLATFORM }} path: ./output @@ -248,7 +248,7 @@ jobs: touch "/tmp/digests/${digest#sha256:}" - name: Upload digest - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v5 if: env.IS_LINUX == 'true' && env.IS_DOCKER_PUSH_CONFIGURED == 'true' && env.IS_ARMV5 == 'false' with: name: digests-${{ env.PLATFORM }} @@ -267,7 +267,7 @@ jobs: - uses: actions/checkout@v5 - name: Download digests - uses: actions/download-artifact@v5 + uses: actions/download-artifact@v6 with: path: /tmp/digests pattern: digests-* @@ -320,7 +320,7 @@ jobs: steps: - uses: actions/checkout@v5 - - uses: actions/download-artifact@v5 + - uses: actions/download-artifact@v6 with: path: ./binaries pattern: navidrome-windows* @@ -339,7 +339,7 @@ jobs: du -h binaries/msi/*.msi - name: Upload MSI files - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v5 with: name: navidrome-windows-installers path: binaries/msi/*.msi @@ -357,7 +357,7 @@ jobs: fetch-depth: 0 fetch-tags: true - - uses: actions/download-artifact@v5 + - uses: actions/download-artifact@v6 with: path: ./binaries pattern: navidrome-* @@ -383,7 +383,7 @@ jobs: rm ./dist/*.tar.gz ./dist/*.zip - name: Upload all-packages artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v5 with: name: packages path: dist/navidrome_0* @@ -406,13 +406,13 @@ jobs: item: ${{ fromJson(needs.release.outputs.package_list) }} steps: - name: Download all-packages artifact - uses: actions/download-artifact@v5 + uses: actions/download-artifact@v6 with: name: packages path: ./dist - name: Upload all-packages artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v5 with: name: navidrome_linux_${{ matrix.item }} path: dist/navidrome_0*_linux_${{ matrix.item }} diff --git a/core/lyrics/sources.go b/core/lyrics/sources.go index 6d4a4cc6f..857dc2eef 100644 --- a/core/lyrics/sources.go +++ b/core/lyrics/sources.go @@ -8,6 +8,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/ioutils" ) func fromEmbedded(ctx context.Context, mf *model.MediaFile) (model.LyricList, error) { @@ -27,8 +28,7 @@ func fromExternalFile(ctx context.Context, mf *model.MediaFile, suffix string) ( externalLyric := basePath[0:len(basePath)-len(ext)] + suffix - contents, err := os.ReadFile(externalLyric) - + contents, err := ioutils.UTF8ReadFile(externalLyric) if errors.Is(err, os.ErrNotExist) { log.Trace(ctx, "no lyrics found at path", "path", externalLyric) return nil, nil diff --git a/core/lyrics/sources_test.go b/core/lyrics/sources_test.go index e92564c00..b3d502101 100644 --- a/core/lyrics/sources_test.go +++ b/core/lyrics/sources_test.go @@ -108,5 +108,39 @@ var _ = Describe("sources", func() { }, })) }) + + It("should handle LRC files with UTF-8 BOM marker (issue #4631)", func() { + // The function looks for , so we need to pass + // a MediaFile with .mp3 path and look for .lrc suffix + mf := model.MediaFile{Path: "tests/fixtures/bom-test.mp3"} + lyrics, err := fromExternalFile(ctx, &mf, ".lrc") + + Expect(err).To(BeNil()) + Expect(lyrics).ToNot(BeNil()) + Expect(lyrics).To(HaveLen(1)) + + // The critical assertion: even with BOM, synced should be true + Expect(lyrics[0].Synced).To(BeTrue(), "Lyrics with BOM marker should be recognized as synced") + Expect(lyrics[0].Line).To(HaveLen(1)) + Expect(lyrics[0].Line[0].Start).To(Equal(gg.P(int64(0)))) + Expect(lyrics[0].Line[0].Value).To(ContainSubstring("作曲")) + }) + + It("should handle UTF-16 LE encoded LRC files", func() { + mf := model.MediaFile{Path: "tests/fixtures/bom-utf16-test.mp3"} + lyrics, err := fromExternalFile(ctx, &mf, ".lrc") + + Expect(err).To(BeNil()) + Expect(lyrics).ToNot(BeNil()) + Expect(lyrics).To(HaveLen(1)) + + // UTF-16 should be properly converted to UTF-8 + Expect(lyrics[0].Synced).To(BeTrue(), "UTF-16 encoded lyrics should be recognized as synced") + Expect(lyrics[0].Line).To(HaveLen(2)) + Expect(lyrics[0].Line[0].Start).To(Equal(gg.P(int64(18800)))) + Expect(lyrics[0].Line[0].Value).To(Equal("We're no strangers to love")) + Expect(lyrics[0].Line[1].Start).To(Equal(gg.P(int64(22801)))) + Expect(lyrics[0].Line[1].Value).To(Equal("You know the rules and so do I")) + }) }) }) diff --git a/core/playlists.go b/core/playlists.go index 2eebc94e7..f98179f88 100644 --- a/core/playlists.go +++ b/core/playlists.go @@ -20,6 +20,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/criteria" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/utils/ioutils" "github.com/navidrome/navidrome/utils/slice" "golang.org/x/text/unicode/norm" ) @@ -97,12 +98,13 @@ func (s *playlists) parsePlaylist(ctx context.Context, playlistFile string, fold } defer file.Close() + reader := ioutils.UTF8Reader(file) extension := strings.ToLower(filepath.Ext(playlistFile)) switch extension { case ".nsp": - err = s.parseNSP(ctx, pls, file) + err = s.parseNSP(ctx, pls, reader) default: - err = s.parseM3U(ctx, pls, folder, file) + err = s.parseM3U(ctx, pls, folder, reader) } return pls, err } diff --git a/core/playlists_test.go b/core/playlists_test.go index 399210ac8..fb42f9c9f 100644 --- a/core/playlists_test.go +++ b/core/playlists_test.go @@ -74,6 +74,24 @@ var _ = Describe("Playlists", func() { Expect(err).ToNot(HaveOccurred()) Expect(pls.Tracks).To(HaveLen(2)) }) + + It("parses playlists with UTF-8 BOM marker", func() { + pls, err := ps.ImportFile(ctx, folder, "bom-test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.OwnerID).To(Equal("123")) + Expect(pls.Name).To(Equal("Test Playlist")) + Expect(pls.Tracks).To(HaveLen(1)) + Expect(pls.Tracks[0].Path).To(Equal("tests/fixtures/playlists/test.mp3")) + }) + + It("parses UTF-16 LE encoded playlists with BOM and converts to UTF-8", func() { + pls, err := ps.ImportFile(ctx, folder, "bom-test-utf16.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.OwnerID).To(Equal("123")) + Expect(pls.Name).To(Equal("UTF-16 Test Playlist")) + Expect(pls.Tracks).To(HaveLen(1)) + Expect(pls.Tracks[0].Path).To(Equal("tests/fixtures/playlists/test.mp3")) + }) }) Describe("NSP", func() { diff --git a/core/share.go b/core/share.go index 202c27d89..eb5e6679b 100644 --- a/core/share.go +++ b/core/share.go @@ -13,6 +13,7 @@ import ( "github.com/navidrome/navidrome/model" . "github.com/navidrome/navidrome/utils/gg" "github.com/navidrome/navidrome/utils/slice" + "github.com/navidrome/navidrome/utils/str" ) type Share interface { @@ -119,9 +120,8 @@ func (r *shareRepositoryWrapper) Save(entity interface{}) (string, error) { log.Error(r.ctx, "Invalid Resource ID", "id", firstId) return "", model.ErrNotFound } - if len(s.Contents) > 30 { - s.Contents = s.Contents[:26] + "..." - } + + s.Contents = str.TruncateRunes(s.Contents, 30, "...") id, err = r.Persistable.Save(s) return id, err diff --git a/core/share_test.go b/core/share_test.go index 21069bb59..475d40ec9 100644 --- a/core/share_test.go +++ b/core/share_test.go @@ -38,6 +38,38 @@ var _ = Describe("Share", func() { Expect(id).ToNot(BeEmpty()) Expect(entity.ID).To(Equal(id)) }) + + It("does not truncate ASCII labels shorter than 30 characters", func() { + _ = ds.MediaFile(ctx).Put(&model.MediaFile{ID: "456", Title: "Example Media File"}) + entity := &model.Share{Description: "test", ResourceIDs: "456"} + _, err := repo.Save(entity) + Expect(err).ToNot(HaveOccurred()) + Expect(entity.Contents).To(Equal("Example Media File")) + }) + + It("truncates ASCII labels longer than 30 characters", func() { + _ = ds.MediaFile(ctx).Put(&model.MediaFile{ID: "789", Title: "Example Media File But The Title Is Really Long For Testing Purposes"}) + entity := &model.Share{Description: "test", ResourceIDs: "789"} + _, err := repo.Save(entity) + Expect(err).ToNot(HaveOccurred()) + Expect(entity.Contents).To(Equal("Example Media File But The ...")) + }) + + It("does not truncate CJK labels shorter than 30 runes", func() { + _ = ds.MediaFile(ctx).Put(&model.MediaFile{ID: "456", Title: "青春コンプレックス"}) + entity := &model.Share{Description: "test", ResourceIDs: "456"} + _, err := repo.Save(entity) + Expect(err).ToNot(HaveOccurred()) + Expect(entity.Contents).To(Equal("青春コンプレックス")) + }) + + It("truncates CJK labels longer than 30 runes", func() { + _ = ds.MediaFile(ctx).Put(&model.MediaFile{ID: "789", Title: "私の中の幻想的世界観及びその顕現を想起させたある現実での出来事に関する一考察"}) + entity := &model.Share{Description: "test", ResourceIDs: "789"} + _, err := repo.Save(entity) + Expect(err).ToNot(HaveOccurred()) + Expect(entity.Contents).To(Equal("私の中の幻想的世界観及びその顕現を想起させたある現実で...")) + }) }) Describe("Update", func() { diff --git a/db/migrations/20250823142158_make_playqueue_position_int.sql b/db/migrations/20250823142158_make_playqueue_position_int.sql new file mode 100644 index 000000000..de20f0c79 --- /dev/null +++ b/db/migrations/20250823142158_make_playqueue_position_int.sql @@ -0,0 +1,9 @@ +-- +goose Up +-- +goose StatementBegin +ALTER TABLE playqueue ADD COLUMN position_int integer; +UPDATE playqueue SET position_int = CAST(position as INTEGER) ; +ALTER TABLE playqueue DROP COLUMN position; +ALTER TABLE playqueue RENAME COLUMN position_int TO position; +-- +goose StatementEnd + +-- +goose Down diff --git a/go.mod b/go.mod index 265cbfa6d..bbe610710 100644 --- a/go.mod +++ b/go.mod @@ -1,9 +1,13 @@ module github.com/navidrome/navidrome -go 1.25.3 +go 1.25.4 -// Fork to fix https://github.com/navidrome/navidrome/pull/3254 -replace github.com/dhowden/tag v0.0.0-20240417053706-3d75831295e8 => github.com/deluan/tag v0.0.0-20241002021117-dfe5e6ea396d +replace ( + // Fork to fix https://github.com/navidrome/navidrome/issues/3254 + github.com/dhowden/tag v0.0.0-20240417053706-3d75831295e8 => github.com/deluan/tag v0.0.0-20241002021117-dfe5e6ea396d + // Using version from main that fixes https://github.com/navidrome/navidrome/issues/4396 + github.com/tetratelabs/wazero v1.9.0 => github.com/tetratelabs/wazero v0.0.0-20251106165119-514cdb337684 +) require ( github.com/Masterminds/squirrel v1.5.4 @@ -43,7 +47,7 @@ require ( github.com/mattn/go-sqlite3 v1.14.32 github.com/microcosm-cc/bluemonday v1.0.27 github.com/mileusna/useragent v1.3.5 - github.com/onsi/ginkgo/v2 v2.27.1 + github.com/onsi/ginkgo/v2 v2.27.2 github.com/onsi/gomega v1.38.2 github.com/pelletier/go-toml/v2 v2.2.4 github.com/pocketbase/dbx v1.11.0 @@ -124,7 +128,6 @@ require ( github.com/stretchr/objx v0.5.2 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/zeebo/xxh3 v1.0.2 // indirect - go.uber.org/automaxprocs v1.6.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect diff --git a/go.sum b/go.sum index f9e620fb2..059ddd19f 100644 --- a/go.sum +++ b/go.sum @@ -186,8 +186,8 @@ github.com/ncruces/go-strftime v0.1.9 h1:bY0MQC28UADQmHmaF5dgpLmImcShSi2kHU9XLdh github.com/ncruces/go-strftime v0.1.9/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= github.com/ogier/pflag v0.0.1 h1:RW6JSWSu/RkSatfcLtogGfFgpim5p7ARQ10ECk5O750= github.com/ogier/pflag v0.0.1/go.mod h1:zkFki7tvTa0tafRvTBIZTvzYyAu6kQhPZFnshFFPE+g= -github.com/onsi/ginkgo/v2 v2.27.1 h1:0LJC8MpUSQnfnp4n/3W3GdlmJP3ENGF0ZPzjQGLPP7s= -github.com/onsi/ginkgo/v2 v2.27.1/go.mod h1:wmy3vCqiBjirARfVhAqFpYt8uvX0yaFe+GudAqqcCqA= +github.com/onsi/ginkgo/v2 v2.27.2 h1:LzwLj0b89qtIy6SSASkzlNvX6WktqurSHwkk2ipF/Ns= +github.com/onsi/ginkgo/v2 v2.27.2/go.mod h1:ArE1D/XhNXBXCBkKOLkbsb2c81dQHCRcF5zwn/ykDRo= github.com/onsi/gomega v1.38.2 h1:eZCjf2xjZAqe+LeWvKb5weQ+NcPwX84kqJ0cZNxok2A= github.com/onsi/gomega v1.38.2/go.mod h1:W2MJcYxRGV63b418Ai34Ud0hEdTVXq9NW9+Sx6uXf3k= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= @@ -201,8 +201,6 @@ github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRI github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pocketbase/dbx v1.11.0 h1:LpZezioMfT3K4tLrqA55wWFw1EtH1pM4tzSVa7kgszU= github.com/pocketbase/dbx v1.11.0/go.mod h1:xXRCIAKTHMgUCyCKZm55pUOdvFziJjQfXaWKhu2vhMs= -github.com/prashantv/gostub v1.1.0 h1:BTyx3RfQjRHnUWaGF9oQos79AlQ5k8WNktv7VGvVH4g= -github.com/prashantv/gostub v1.1.0/go.mod h1:A5zLQHz7ieHGG7is6LLXLz7I8+3LZzsrV0P1IAHhP5U= github.com/pressly/goose/v3 v3.26.0 h1:KJakav68jdH0WDvoAcj8+n61WqOIaPGgH0bJWS6jpmM= github.com/pressly/goose/v3 v3.26.0/go.mod h1:4hC1KrritdCxtuFsqgs1R4AU5bWtTAf+cnWvfhf2DNY= github.com/prometheus/client_golang v1.23.2 h1:Je96obch5RDVy3FDMndoUsjAhG5Edi49h0RJWRi/o0o= @@ -267,8 +265,8 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= -github.com/tetratelabs/wazero v1.9.0 h1:IcZ56OuxrtaEz8UYNRHBrUa9bYeX9oVY93KspZZBf/I= -github.com/tetratelabs/wazero v1.9.0/go.mod h1:TSbcXCfFP0L2FGkRPxHphadXPjo1T6W+CseNNY7EkjM= +github.com/tetratelabs/wazero v0.0.0-20251106165119-514cdb337684 h1:ugT1JTRsK1Jhn95BWilCugyZ1Svsyxm9xSiflOa2e7E= +github.com/tetratelabs/wazero v0.0.0-20251106165119-514cdb337684/go.mod h1:DRm5twOQ5Gr1AoEdSi0CLjDQF1J9ZAuyqFIjl1KKfQU= github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= @@ -286,8 +284,6 @@ github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ= github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0= github.com/zeebo/xxh3 v1.0.2/go.mod h1:5NWz9Sef7zIDm2JHfFlcQvNekmcEl9ekUZQQKCYaDcA= -go.uber.org/automaxprocs v1.6.0 h1:O3y2/QNTOdbF+e/dpXNNW7Rx2hZ4sTIPyybbxyNqTUs= -go.uber.org/automaxprocs v1.6.0/go.mod h1:ifeIMSnPZuznNm6jmdzmU3/bfk01Fe2fotchwEFJ8r8= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index 4be89bcb8..a062b4398 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -55,6 +55,7 @@ var _ = Describe("AlbumRepository", func() { It("returns all records sorted", func() { Expect(GetAll(model.QueryOptions{Sort: "name"})).To(Equal(model.Albums{ albumAbbeyRoad, + albumMultiDisc, albumRadioactivity, albumSgtPeppers, })) @@ -64,6 +65,7 @@ var _ = Describe("AlbumRepository", func() { Expect(GetAll(model.QueryOptions{Sort: "name", Order: "desc"})).To(Equal(model.Albums{ albumSgtPeppers, albumRadioactivity, + albumMultiDisc, albumAbbeyRoad, })) }) diff --git a/persistence/artist_repository.go b/persistence/artist_repository.go index a7cf9272a..6d08c27db 100644 --- a/persistence/artist_repository.go +++ b/persistence/artist_repository.go @@ -400,23 +400,16 @@ func (r *artistRepository) RefreshStats(allArtists bool) (int64, error) { // This now calculates per-library statistics and stores them in library_artist.stats batchUpdateStatsSQL := ` WITH artist_role_counters AS ( - SELECT jt.atom AS artist_id, + SELECT mfa.artist_id, mf.library_id, - substr( - replace(jt.path, '$.', ''), - 1, - CASE WHEN instr(replace(jt.path, '$.', ''), '[') > 0 - THEN instr(replace(jt.path, '$.', ''), '[') - 1 - ELSE length(replace(jt.path, '$.', '')) - END - ) AS role, + mfa.role, count(DISTINCT mf.album_id) AS album_count, - count(mf.id) AS count, + count(DISTINCT mf.id) AS count, sum(mf.size) AS size - FROM media_file mf - JOIN json_tree(mf.participants) jt ON jt.key = 'id' AND jt.atom IS NOT NULL - WHERE jt.atom IN (ROLE_IDS_PLACEHOLDER) -- Will replace with actual placeholders - GROUP BY jt.atom, mf.library_id, role + FROM media_file_artists mfa + JOIN media_file mf ON mfa.media_file_id = mf.id + WHERE mfa.artist_id IN (ROLE_IDS_PLACEHOLDER) -- Will replace with actual placeholders + GROUP BY mfa.artist_id, mf.library_id, mfa.role ), artist_total_counters AS ( SELECT mfa.artist_id, @@ -445,24 +438,24 @@ func (r *artistRepository) RefreshStats(allArtists bool) (int64, error) { ), combined_counters AS ( SELECT artist_id, library_id, role, album_count, count, size FROM artist_role_counters - UNION + UNION ALL SELECT artist_id, library_id, role, album_count, count, size FROM artist_total_counters - UNION + UNION ALL SELECT artist_id, library_id, role, album_count, count, size FROM artist_participant_counter ), library_artist_counters AS ( SELECT artist_id, library_id, json_group_object( - replace(role, '"', ''), + role, json_object('a', album_count, 'm', count, 's', size) ) AS counters FROM combined_counters GROUP BY artist_id, library_id ) UPDATE library_artist - SET stats = coalesce((SELECT counters FROM library_artist_counters lac - WHERE lac.artist_id = library_artist.artist_id + SET stats = coalesce((SELECT counters FROM library_artist_counters lac + WHERE lac.artist_id = library_artist.artist_id AND lac.library_id = library_artist.library_id), '{}') WHERE library_artist.artist_id IN (ROLE_IDS_PLACEHOLDER);` // Will replace with actual placeholders diff --git a/persistence/mediafile_repository_test.go b/persistence/mediafile_repository_test.go index 002b82499..ab926c00d 100644 --- a/persistence/mediafile_repository_test.go +++ b/persistence/mediafile_repository_test.go @@ -38,7 +38,7 @@ var _ = Describe("MediaRepository", func() { }) It("counts the number of mediafiles in the DB", func() { - Expect(mr.CountAll()).To(Equal(int64(6))) + Expect(mr.CountAll()).To(Equal(int64(10))) }) It("returns songs ordered by lyrics with a specific title/artist", func() { diff --git a/persistence/persistence_suite_test.go b/persistence/persistence_suite_test.go index 1007d84fe..f3cb4f3d0 100644 --- a/persistence/persistence_suite_test.go +++ b/persistence/persistence_suite_test.go @@ -69,10 +69,12 @@ var ( albumSgtPeppers = al(model.Album{ID: "101", Name: "Sgt Peppers", AlbumArtist: "The Beatles", OrderAlbumName: "sgt peppers", AlbumArtistID: "3", EmbedArtPath: p("/beatles/1/sgt/a day.mp3"), SongCount: 1, MaxYear: 1967}) albumAbbeyRoad = al(model.Album{ID: "102", Name: "Abbey Road", AlbumArtist: "The Beatles", OrderAlbumName: "abbey road", AlbumArtistID: "3", EmbedArtPath: p("/beatles/1/come together.mp3"), SongCount: 1, MaxYear: 1969}) albumRadioactivity = al(model.Album{ID: "103", Name: "Radioactivity", AlbumArtist: "Kraftwerk", OrderAlbumName: "radioactivity", AlbumArtistID: "2", EmbedArtPath: p("/kraft/radio/radio.mp3"), SongCount: 2}) + albumMultiDisc = al(model.Album{ID: "104", Name: "Multi Disc Album", AlbumArtist: "Test Artist", OrderAlbumName: "multi disc album", AlbumArtistID: "1", EmbedArtPath: p("/test/multi/disc1/track1.mp3"), SongCount: 4}) testAlbums = model.Albums{ albumSgtPeppers, albumAbbeyRoad, albumRadioactivity, + albumMultiDisc, } ) @@ -94,13 +96,22 @@ var ( Lyrics: `[{"lang":"xxx","line":[{"value":"This is a set of lyrics"}],"synced":false}]`, }) songAntenna2 = mf(model.MediaFile{ID: "1006", Title: "Antenna", ArtistID: "2", Artist: "Kraftwerk", AlbumID: "103"}) - testSongs = model.MediaFiles{ + // Multi-disc album tracks (intentionally out of order to test sorting) + songDisc2Track11 = mf(model.MediaFile{ID: "2001", Title: "Disc 2 Track 11", ArtistID: "1", Artist: "Test Artist", AlbumID: "104", Album: "Multi Disc Album", DiscNumber: 2, TrackNumber: 11, Path: p("/test/multi/disc2/track11.mp3"), OrderAlbumName: "multi disc album", OrderArtistName: "test artist"}) + songDisc1Track01 = mf(model.MediaFile{ID: "2002", Title: "Disc 1 Track 1", ArtistID: "1", Artist: "Test Artist", AlbumID: "104", Album: "Multi Disc Album", DiscNumber: 1, TrackNumber: 1, Path: p("/test/multi/disc1/track1.mp3"), OrderAlbumName: "multi disc album", OrderArtistName: "test artist"}) + songDisc2Track01 = mf(model.MediaFile{ID: "2003", Title: "Disc 2 Track 1", ArtistID: "1", Artist: "Test Artist", AlbumID: "104", Album: "Multi Disc Album", DiscNumber: 2, TrackNumber: 1, Path: p("/test/multi/disc2/track1.mp3"), OrderAlbumName: "multi disc album", OrderArtistName: "test artist"}) + songDisc1Track02 = mf(model.MediaFile{ID: "2004", Title: "Disc 1 Track 2", ArtistID: "1", Artist: "Test Artist", AlbumID: "104", Album: "Multi Disc Album", DiscNumber: 1, TrackNumber: 2, Path: p("/test/multi/disc1/track2.mp3"), OrderAlbumName: "multi disc album", OrderArtistName: "test artist"}) + testSongs = model.MediaFiles{ songDayInALife, songComeTogether, songRadioactivity, songAntenna, songAntennaWithLyrics, songAntenna2, + songDisc2Track11, + songDisc1Track01, + songDisc2Track01, + songDisc1Track02, } ) diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go index 15ae438d9..7fad93b1e 100644 --- a/persistence/playlist_repository_test.go +++ b/persistence/playlist_repository_test.go @@ -219,4 +219,37 @@ var _ = Describe("PlaylistRepository", func() { }) }) }) + + Describe("Playlist Track Sorting", func() { + var testPlaylistID string + + AfterEach(func() { + if testPlaylistID != "" { + Expect(repo.Delete(testPlaylistID)).To(BeNil()) + testPlaylistID = "" + } + }) + + It("sorts tracks correctly by album (disc and track number)", func() { + By("creating a playlist with multi-disc album tracks in arbitrary order") + newPls := model.Playlist{Name: "Multi-Disc Test", OwnerID: "userid"} + // Add tracks in intentionally scrambled order + newPls.AddMediaFilesByID([]string{"2001", "2002", "2003", "2004"}) + Expect(repo.Put(&newPls)).To(Succeed()) + testPlaylistID = newPls.ID + + By("retrieving tracks sorted by album") + tracksRepo := repo.Tracks(newPls.ID, false) + tracks, err := tracksRepo.GetAll(model.QueryOptions{Sort: "album", Order: "asc"}) + Expect(err).ToNot(HaveOccurred()) + + By("verifying tracks are sorted by disc number then track number") + Expect(tracks).To(HaveLen(4)) + // Expected order: Disc 1 Track 1, Disc 1 Track 2, Disc 2 Track 1, Disc 2 Track 11 + Expect(tracks[0].MediaFileID).To(Equal("2002")) // Disc 1, Track 1 + Expect(tracks[1].MediaFileID).To(Equal("2004")) // Disc 1, Track 2 + Expect(tracks[2].MediaFileID).To(Equal("2003")) // Disc 2, Track 1 + Expect(tracks[3].MediaFileID).To(Equal("2001")) // Disc 2, Track 11 + }) + }) }) diff --git a/persistence/playlist_track_repository.go b/persistence/playlist_track_repository.go index 01eec0d02..b3f9e0c07 100644 --- a/persistence/playlist_track_repository.go +++ b/persistence/playlist_track_repository.go @@ -55,7 +55,7 @@ func (r *playlistRepository) Tracks(playlistId string, refreshSmartPlaylist bool "id": "playlist_tracks.id", "artist": "order_artist_name", "album_artist": "order_album_artist_name", - "album": "order_album_name, order_album_artist_name", + "album": "order_album_name, album_id, disc_number, track_number, order_artist_name, title", "title": "order_title", // To make sure these fields will be whitelisted "duration": "duration", diff --git a/persistence/user_repository.go b/persistence/user_repository.go index a7181b1a7..7baa8f6a8 100644 --- a/persistence/user_repository.go +++ b/persistence/user_repository.go @@ -57,6 +57,7 @@ func NewUserRepository(ctx context.Context, db dbx.Builder) model.UserRepository r.db = db r.tableName = "user" r.registerModel(&model.User{}, map[string]filterFunc{ + "id": idFilter(r.tableName), "password": invalidFilter(ctx), "name": r.withTableName(startsWithFilter), }) diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go index 7c0707ecd..8abbf76a9 100644 --- a/persistence/user_repository_test.go +++ b/persistence/user_repository_test.go @@ -559,4 +559,15 @@ var _ = Describe("UserRepository", func() { Expect(user.Libraries[0].ID).To(Equal(1)) }) }) + + Describe("filters", func() { + It("qualifies id filter with table name", func() { + r := repo.(*userRepository) + qo := r.parseRestOptions(r.ctx, rest.QueryOptions{Filters: map[string]any{"id": "123"}}) + sel := r.selectUserWithLibraries(qo) + query, _, err := r.toSQL(sel) + Expect(err).NotTo(HaveOccurred()) + Expect(query).To(ContainSubstring("user.id = {:p0}")) + }) + }) }) diff --git a/tests/fixtures/bom-test.lrc b/tests/fixtures/bom-test.lrc new file mode 100644 index 000000000..223c37de0 --- /dev/null +++ b/tests/fixtures/bom-test.lrc @@ -0,0 +1,4 @@ +[00:00.00] 作曲 : 柏大輔 +NOTE: This file intentionally contains a UTF-8 BOM (Byte Order Mark) at byte 0. +This tests BOM handling in lyrics parsing (GitHub issue #4631). +The BOM bytes are: 0xEF 0xBB 0xBF \ No newline at end of file diff --git a/tests/fixtures/bom-utf16-test.lrc b/tests/fixtures/bom-utf16-test.lrc new file mode 100644 index 000000000..e40ea3255 Binary files /dev/null and b/tests/fixtures/bom-utf16-test.lrc differ diff --git a/tests/fixtures/playlists/bom-test-utf16.m3u b/tests/fixtures/playlists/bom-test-utf16.m3u new file mode 100644 index 000000000..9c2e9d599 Binary files /dev/null and b/tests/fixtures/playlists/bom-test-utf16.m3u differ diff --git a/tests/fixtures/playlists/bom-test.m3u b/tests/fixtures/playlists/bom-test.m3u new file mode 100644 index 000000000..f5a00806c --- /dev/null +++ b/tests/fixtures/playlists/bom-test.m3u @@ -0,0 +1,6 @@ +#EXTM3U +# NOTE: This file intentionally contains a UTF-8 BOM (Byte Order Mark) at the beginning +# (bytes 0xEF 0xBB 0xBF) to test BOM handling in playlist parsing. +#PLAYLIST:Test Playlist +#EXTINF:123,Test Artist - Test Song +test.mp3 diff --git a/ui/src/audioplayer/Player.jsx b/ui/src/audioplayer/Player.jsx index c5a9885ae..2c9034877 100644 --- a/ui/src/audioplayer/Player.jsx +++ b/ui/src/audioplayer/Player.jsx @@ -118,6 +118,7 @@ const Player = () => { /> ), locale: locale(translate), + sortableOptions: { delay: 200, delayOnTouchOnly: true }, }), [playerTheme, playerState.mode, isDesktop, gainInfo, translate], ) diff --git a/ui/src/themes/gruvboxDark.js b/ui/src/themes/gruvboxDark.js index b576e7713..b1a2e4c90 100644 --- a/ui/src/themes/gruvboxDark.js +++ b/ui/src/themes/gruvboxDark.js @@ -40,6 +40,11 @@ export default { color: '#ebdbb2', }, }, + MuiIconButton: { + root: { + color: '#ebdbb2', + }, + }, MuiChip: { clickable: { background: '#49483e', diff --git a/ui/src/themes/ligera.js b/ui/src/themes/ligera.js index 824cf7e67..0ef1601a2 100644 --- a/ui/src/themes/ligera.js +++ b/ui/src/themes/ligera.js @@ -450,13 +450,21 @@ export default { }, RaPaginationActions: { button: { - backgroundColor: 'inherit', + backgroundColor: '#fff', + color: '#000', minWidth: 48, margin: '0 4px', - border: '1px solid #282828', + border: '1px solid #cccccc', '@global': { '> .MuiButton-label': { padding: 0, + color: '#656565', + '&:hover': { + color: '#fff !important', + }, + }, + '> .MuiButton-label > svg': { + color: '#656565', }, }, }, diff --git a/ui/src/utils/formatters.js b/ui/src/utils/formatters.js index 74cce6e15..cfcb84b05 100644 --- a/ui/src/utils/formatters.js +++ b/ui/src/utils/formatters.js @@ -95,7 +95,7 @@ export const formatFullDate = (date, locale) => { return new Date(date).toLocaleDateString(locale, options) } -export const formatNumber = (value) => { +export const formatNumber = (value, locale) => { if (value === null || value === undefined) return '0' - return value.toLocaleString() + return value.toLocaleString(locale) } diff --git a/ui/src/utils/formatters.test.js b/ui/src/utils/formatters.test.js index 7709dd91b..d633e96f2 100644 --- a/ui/src/utils/formatters.test.js +++ b/ui/src/utils/formatters.test.js @@ -121,35 +121,35 @@ describe('formatDuration2', () => { describe('formatNumber', () => { it('handles null and undefined values', () => { - expect(formatNumber(null)).toEqual('0') - expect(formatNumber(undefined)).toEqual('0') + expect(formatNumber(null, 'en-CA')).toEqual('0') + expect(formatNumber(undefined, 'en-CA')).toEqual('0') }) it('formats integers', () => { - expect(formatNumber(0)).toEqual('0') - expect(formatNumber(1)).toEqual('1') - expect(formatNumber(123)).toEqual('123') - expect(formatNumber(1000)).toEqual('1,000') - expect(formatNumber(1234567)).toEqual('1,234,567') + expect(formatNumber(0, 'en-CA')).toEqual('0') + expect(formatNumber(1, 'en-CA')).toEqual('1') + expect(formatNumber(123, 'en-CA')).toEqual('123') + expect(formatNumber(1000, 'en-CA')).toEqual('1,000') + expect(formatNumber(1234567, 'en-CA')).toEqual('1,234,567') }) it('formats decimal numbers', () => { - expect(formatNumber(123.45)).toEqual('123.45') - expect(formatNumber(1234.567)).toEqual('1,234.567') + expect(formatNumber(123.45, 'en-CA')).toEqual('123.45') + expect(formatNumber(1234.567, 'en-CA')).toEqual('1,234.567') }) it('formats negative numbers', () => { - expect(formatNumber(-123)).toEqual('-123') - expect(formatNumber(-1234)).toEqual('-1,234') - expect(formatNumber(-123.45)).toEqual('-123.45') + expect(formatNumber(-123, 'en-CA')).toEqual('-123') + expect(formatNumber(-1234, 'en-CA')).toEqual('-1,234') + expect(formatNumber(-123.45, 'en-CA')).toEqual('-123.45') }) }) describe('formatFullDate', () => { it('format dates', () => { - expect(formatFullDate('2011', 'en-US')).toEqual('2011') - expect(formatFullDate('2011-06', 'en-US')).toEqual('Jun 2011') - expect(formatFullDate('1985-01-01', 'en-US')).toEqual('Jan 1, 1985') + expect(formatFullDate('2011', 'en-CA')).toEqual('2011') + expect(formatFullDate('2011-06', 'en-CA')).toEqual('Jun 2011') + expect(formatFullDate('1985-01-01', 'en-CA')).toEqual('Jan 1, 1985') expect(formatFullDate('199704')).toEqual('') }) }) diff --git a/utils/ioutils/ioutils.go b/utils/ioutils/ioutils.go new file mode 100644 index 000000000..89d3997f3 --- /dev/null +++ b/utils/ioutils/ioutils.go @@ -0,0 +1,33 @@ +package ioutils + +import ( + "io" + "os" + + "golang.org/x/text/encoding/unicode" + "golang.org/x/text/transform" +) + +// UTF8Reader wraps an io.Reader to handle Byte Order Mark (BOM) properly. +// It strips UTF-8 BOM if present, and converts UTF-16 (LE/BE) to UTF-8. +// This is particularly useful for reading user-provided text files (like LRC lyrics, +// playlists) that may have been created on Windows, which often adds BOM markers. +// +// Reference: https://en.wikipedia.org/wiki/Byte_order_mark +func UTF8Reader(r io.Reader) io.Reader { + return transform.NewReader(r, unicode.BOMOverride(unicode.UTF8.NewDecoder())) +} + +// UTF8ReadFile reads the named file and returns its contents as a byte slice, +// automatically handling BOM markers. It's similar to os.ReadFile but strips +// UTF-8 BOM and converts UTF-16 encoded files to UTF-8. +func UTF8ReadFile(filename string) ([]byte, error) { + file, err := os.Open(filename) + if err != nil { + return nil, err + } + defer file.Close() + + reader := UTF8Reader(file) + return io.ReadAll(reader) +} diff --git a/utils/ioutils/ioutils_test.go b/utils/ioutils/ioutils_test.go new file mode 100644 index 000000000..7f5483879 --- /dev/null +++ b/utils/ioutils/ioutils_test.go @@ -0,0 +1,117 @@ +package ioutils + +import ( + "bytes" + "io" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestIOUtils(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "IO Utils Suite") +} + +var _ = Describe("UTF8Reader", func() { + Context("when reading text with UTF-8 BOM", func() { + It("strips the UTF-8 BOM marker", func() { + // UTF-8 BOM is EF BB BF + input := []byte{0xEF, 0xBB, 0xBF, 'h', 'e', 'l', 'l', 'o'} + reader := UTF8Reader(bytes.NewReader(input)) + + output, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(output)).To(Equal("hello")) + }) + + It("strips UTF-8 BOM from multi-line text", func() { + // Test with the actual LRC file format + input := []byte{0xEF, 0xBB, 0xBF, '[', '0', '0', ':', '0', '0', '.', '0', '0', ']', ' ', 't', 'e', 's', 't'} + reader := UTF8Reader(bytes.NewReader(input)) + + output, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(output)).To(Equal("[00:00.00] test")) + }) + }) + + Context("when reading text without BOM", func() { + It("passes through unchanged", func() { + input := []byte("hello world") + reader := UTF8Reader(bytes.NewReader(input)) + + output, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(output)).To(Equal("hello world")) + }) + }) + + Context("when reading UTF-16 LE encoded text", func() { + It("converts to UTF-8 and strips BOM", func() { + // UTF-16 LE BOM (FF FE) followed by "hi" in UTF-16 LE + input := []byte{0xFF, 0xFE, 'h', 0x00, 'i', 0x00} + reader := UTF8Reader(bytes.NewReader(input)) + + output, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(output)).To(Equal("hi")) + }) + }) + + Context("when reading UTF-16 BE encoded text", func() { + It("converts to UTF-8 and strips BOM", func() { + // UTF-16 BE BOM (FE FF) followed by "hi" in UTF-16 BE + input := []byte{0xFE, 0xFF, 0x00, 'h', 0x00, 'i'} + reader := UTF8Reader(bytes.NewReader(input)) + + output, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(output)).To(Equal("hi")) + }) + }) + + Context("when reading empty content", func() { + It("returns empty string", func() { + reader := UTF8Reader(bytes.NewReader([]byte{})) + + output, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(output)).To(Equal("")) + }) + }) +}) + +var _ = Describe("UTF8ReadFile", func() { + Context("when reading a file with UTF-8 BOM", func() { + It("strips the BOM marker", func() { + // Use the actual fixture from issue #4631 + contents, err := UTF8ReadFile("../../tests/fixtures/bom-test.lrc") + Expect(err).ToNot(HaveOccurred()) + + // Should NOT start with BOM + Expect(contents[0]).ToNot(Equal(byte(0xEF))) + // Should start with '[' + Expect(contents[0]).To(Equal(byte('['))) + Expect(string(contents)).To(HavePrefix("[00:00.00]")) + }) + }) + + Context("when reading a file without BOM", func() { + It("reads the file normally", func() { + contents, err := UTF8ReadFile("../../tests/fixtures/test.lrc") + Expect(err).ToNot(HaveOccurred()) + + // Should contain the expected content + Expect(string(contents)).To(ContainSubstring("We're no strangers to love")) + }) + }) + + Context("when reading a non-existent file", func() { + It("returns an error", func() { + _, err := UTF8ReadFile("../../tests/fixtures/nonexistent.lrc") + Expect(err).To(HaveOccurred()) + }) + }) +}) diff --git a/utils/str/str.go b/utils/str/str.go index 8a94488de..f662473da 100644 --- a/utils/str/str.go +++ b/utils/str/str.go @@ -2,6 +2,7 @@ package str import ( "strings" + "unicode/utf8" ) var utf8ToAscii = func() *strings.Replacer { @@ -39,3 +40,25 @@ func LongestCommonPrefix(list []string) string { } return list[0] } + +// TruncateRunes truncates a string to a maximum number of runes, adding a suffix if truncated. +// The suffix is included in the rune count, so if maxRunes is 30 and suffix is "...", the actual +// string content will be truncated to fit within the maxRunes limit including the suffix. +func TruncateRunes(s string, maxRunes int, suffix string) string { + if utf8.RuneCountInString(s) <= maxRunes { + return s + } + + suffixRunes := utf8.RuneCountInString(suffix) + truncateAt := maxRunes - suffixRunes + if truncateAt < 0 { + truncateAt = 0 + } + + runes := []rune(s) + if truncateAt >= len(runes) { + return s + suffix + } + + return string(runes[:truncateAt]) + suffix +} diff --git a/utils/str/str_test.go b/utils/str/str_test.go index 0c3524e4e..511805831 100644 --- a/utils/str/str_test.go +++ b/utils/str/str_test.go @@ -31,6 +31,72 @@ var _ = Describe("String Utils", func() { Expect(str.LongestCommonPrefix(albums)).To(Equal("/artist/album")) }) }) + + Describe("TruncateRunes", func() { + It("returns string unchanged if under max runes", func() { + Expect(str.TruncateRunes("hello", 10, "...")).To(Equal("hello")) + }) + + It("returns string unchanged if exactly at max runes", func() { + Expect(str.TruncateRunes("hello", 5, "...")).To(Equal("hello")) + }) + + It("truncates and adds suffix when over max runes", func() { + Expect(str.TruncateRunes("hello world", 8, "...")).To(Equal("hello...")) + }) + + It("handles unicode characters correctly", func() { + // 6 emoji characters, maxRunes=5, suffix="..." (3 runes) + // So content gets 5-3=2 runes + Expect(str.TruncateRunes("😀😁😂😃😄😅", 5, "...")).To(Equal("😀😁...")) + }) + + It("handles multi-byte UTF-8 characters", func() { + // Characters like é are single runes + Expect(str.TruncateRunes("Café au Lait", 5, "...")).To(Equal("Ca...")) + }) + + It("works with empty suffix", func() { + Expect(str.TruncateRunes("hello world", 5, "")).To(Equal("hello")) + }) + + It("accounts for suffix length in truncation", func() { + // maxRunes=10, suffix="..." (3 runes) -> leaves 7 runes for content + result := str.TruncateRunes("hello world this is long", 10, "...") + Expect(result).To(Equal("hello w...")) + // Verify total rune count is <= maxRunes + runeCount := len([]rune(result)) + Expect(runeCount).To(BeNumerically("<=", 10)) + }) + + It("handles very long suffix gracefully", func() { + // If suffix is longer than maxRunes, we still add it + // but the content will be truncated to 0 + result := str.TruncateRunes("hello world", 5, "... (truncated)") + // Result will be just the suffix (since truncateAt=0) + Expect(result).To(Equal("... (truncated)")) + }) + + It("handles empty string", func() { + Expect(str.TruncateRunes("", 10, "...")).To(Equal("")) + }) + + It("uses custom suffix", func() { + // maxRunes=11, suffix=" [...]" (6 runes) -> content gets 5 runes + // "hello world" is 11 runes exactly, so we need a longer string + Expect(str.TruncateRunes("hello world extra", 11, " [...]")).To(Equal("hello [...]")) + }) + + DescribeTable("truncates at rune boundaries (not byte boundaries)", + func(input string, maxRunes int, suffix string, expected string) { + Expect(str.TruncateRunes(input, maxRunes, suffix)).To(Equal(expected)) + }, + Entry("ASCII", "abcdefghij", 5, "...", "ab..."), + Entry("Mixed ASCII and Unicode", "ab😀cd", 4, ".", "ab😀."), + Entry("All emoji", "😀😁😂😃😄", 3, "…", "😀😁…"), + Entry("Japanese", "こんにちは世界", 3, "…", "こん…"), + ) + }) }) var testPaths = []string{