diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 6ebb579e8..09fca2572 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -120,6 +120,79 @@ jobs: go build -o ndpgen . ./ndpgen --help + go-windows: + name: Test Go code (Windows) + runs-on: windows-2022 + env: + FFMPEG_VERSION: "7.1" + FFMPEG_REPOSITORY: navidrome/ffmpeg-windows-builds + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + + - uses: msys2/setup-msys2@v2 + with: + msystem: MINGW64 + install: mingw-w64-x86_64-gcc + update: false + + - name: Add mingw64 to PATH + shell: bash + run: echo "C:/msys64/mingw64/bin" >> $GITHUB_PATH + + - name: Cache ffmpeg + id: ffmpeg-cache + uses: actions/cache@v4 + with: + path: C:\ffmpeg + key: ffmpeg-${{ env.FFMPEG_VERSION }}-win64 + + - name: Download ffmpeg + if: steps.ffmpeg-cache.outputs.cache-hit != 'true' + shell: pwsh + run: | + $asset = "ffmpeg-n${env:FFMPEG_VERSION}-latest-win64-gpl-${env:FFMPEG_VERSION}" + $url = "https://github.com/${env:FFMPEG_REPOSITORY}/releases/download/latest/$asset.zip" + Invoke-WebRequest -Uri $url -OutFile ffmpeg.zip + Expand-Archive ffmpeg.zip -DestinationPath C:\ffmpeg-extracted + New-Item -ItemType Directory -Force -Path C:\ffmpeg\bin | Out-Null + Copy-Item "C:\ffmpeg-extracted\$asset\bin\ffmpeg.exe" C:\ffmpeg\bin + Copy-Item "C:\ffmpeg-extracted\$asset\bin\ffprobe.exe" C:\ffmpeg\bin + + - name: Add ffmpeg to PATH + shell: bash + run: echo "C:/ffmpeg/bin" >> $GITHUB_PATH + + - name: Verify toolchain + shell: pwsh + run: | + go version + where.exe gcc + gcc --version + ffmpeg -version + ffprobe -version + + - name: Download dependencies + shell: bash + run: go mod download + + - name: Test + shell: bash + env: + CGO_ENABLED: "1" + run: go test -shuffle=on -tags netgo,sqlite_fts5 ./... -v + + - name: Test ndpgen + shell: pwsh + run: | + cd plugins\cmd\ndpgen + go test -shuffle=on -v + go build -o ndpgen.exe . + .\ndpgen.exe --help + js: name: Test JS code runs-on: ubuntu-latest @@ -184,7 +257,7 @@ jobs: build: name: Build - needs: [js, go, go-lint, i18n-lint, git-version, check-push-enabled] + needs: [js, go, go-windows, go-lint, i18n-lint, git-version, check-push-enabled] strategy: matrix: platform: [ linux/amd64, linux/arm64, linux/arm/v5, linux/arm/v6, linux/arm/v7, linux/386, linux/riscv64, darwin/amd64, darwin/arm64, windows/amd64, windows/386 ] diff --git a/Makefile b/Makefile index 4efbc4db6..ad96afd31 100644 --- a/Makefile +++ b/Makefile @@ -75,8 +75,8 @@ test-i18n: ##@Development Validate all translations files install-golangci-lint: ##@Development Install golangci-lint if not present @INSTALL=false; \ - if PATH=$$PATH:./bin which golangci-lint > /dev/null 2>&1; then \ - CURRENT_VERSION=$$(PATH=$$PATH:./bin golangci-lint version 2>/dev/null | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -n1); \ + if PATH=./bin:$$PATH which golangci-lint > /dev/null 2>&1; then \ + CURRENT_VERSION=$$(PATH=./bin:$$PATH golangci-lint version 2>/dev/null | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -n1); \ REQUIRED_VERSION=$$(echo "$(GOLANGCI_LINT_VERSION)" | sed 's/^v//'); \ if [ "$$CURRENT_VERSION" != "$$REQUIRED_VERSION" ]; then \ echo "Found golangci-lint $$CURRENT_VERSION, but $$REQUIRED_VERSION is required. Reinstalling..."; \ @@ -93,7 +93,7 @@ install-golangci-lint: ##@Development Install golangci-lint if not present .PHONY: install-golangci-lint lint: install-golangci-lint ##@Development Lint Go code - PATH=$$PATH:./bin golangci-lint run --timeout 5m + PATH=./bin:$$PATH golangci-lint run --timeout 5m .PHONY: lint lintall: lint ##@Development Lint Go and JS code diff --git a/adapters/gotaglib/gotaglib_test.go b/adapters/gotaglib/gotaglib_test.go index 6756fb690..05924914d 100644 --- a/adapters/gotaglib/gotaglib_test.go +++ b/adapters/gotaglib/gotaglib_test.go @@ -5,6 +5,7 @@ import ( "os" "strings" + "github.com/navidrome/navidrome/tests" "github.com/navidrome/navidrome/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -213,6 +214,7 @@ var _ = Describe("Extractor", func() { // Only run permission tests if we are not root RegularUserContext("when run without root privileges", func() { BeforeEach(func() { + tests.SkipOnWindows("uses Unix file permission bits") // Use root fs for absolute paths in temp directory e = &extractor{fs: os.DirFS("/")} accessForbiddenFile = utils.TempFileName("access_forbidden-", ".mp3") diff --git a/conf/configuration.go b/conf/configuration.go index 89d53d095..9ee77c8bf 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -27,6 +27,7 @@ type configOptions struct { Address string Port int UnixSocketPerm string + EnforceNonRootUser bool MusicFolder string DataFolder string CacheFolder string @@ -60,8 +61,8 @@ type configOptions struct { SmartPlaylistRefreshDelay time.Duration AutoTranscodeDownload bool DefaultDownsamplingFormat string - Search searchOptions `json:",omitzero"` - SimilarSongsMatchThreshold int + Search searchOptions `json:",omitzero"` + Matcher matcherOptions `json:",omitzero"` RecentlyAddedByModTime bool PreferSortTags bool IgnoredArticles string @@ -262,6 +263,11 @@ type searchOptions struct { FullString bool } +type matcherOptions struct { + PreferStarred bool + FuzzyThreshold int +} + // logFatal prints a fatal error message to stderr and exits. // Overridden in tests to allow testing fatal paths. var logFatal = func(args ...any) { @@ -269,6 +275,12 @@ var logFatal = func(args ...any) { os.Exit(1) } +var getEUID = os.Geteuid + +var currentGOOS = func() string { + return runtime.GOOS +} + var ( Server = &configOptions{} hooks []func() @@ -292,12 +304,18 @@ func Load(noConfigDump bool) { mapDeprecatedOption("ReverseProxyUserHeader", "ExtAuth.UserHeader") mapDeprecatedOption("HTTPSecurityHeaders.CustomFrameOptionsValue", "HTTPHeaders.FrameOptions") mapDeprecatedOption("CoverJpegQuality", "CoverArtQuality") + mapDeprecatedOption("SimilarSongsMatchThreshold", "Matcher.FuzzyThreshold") err := viper.Unmarshal(&Server) if err != nil { logFatal("Error parsing config:", err) } + // Validate non-root user early, before any filesystem operations + if err := validateEnforceNonRootUser(); err != nil { + logFatal(err) + } + err = os.MkdirAll(Server.DataFolder, os.ModePerm) if err != nil { logFatal("Error creating data path:", err) @@ -425,6 +443,7 @@ func Load(noConfigDump bool) { logDeprecatedOptions("ReverseProxyUserHeader", "ExtAuth.UserHeader") logDeprecatedOptions("HTTPSecurityHeaders.CustomFrameOptionsValue", "HTTPHeaders.FrameOptions") logDeprecatedOptions("CoverJpegQuality", "CoverArtQuality") + logDeprecatedOptions("SimilarSongsMatchThreshold", "Matcher.FuzzyThreshold") // Removed options logRemovedOptions("Spotify.ID", "Spotify.Secret") @@ -593,6 +612,18 @@ func validateMaxImageUploadSize() error { return nil } +func validateEnforceNonRootUser() error { + if !Server.EnforceNonRootUser || currentGOOS() == "windows" { + return nil + } + + if getEUID() == 0 { + return fmt.Errorf("EnforceNonRootUser is enabled but Navidrome is running as root") + } + + return nil +} + func validateScanSchedule() error { if Server.Scanner.Schedule == "0" || Server.Scanner.Schedule == "" { Server.Scanner.Schedule = "" @@ -692,6 +723,7 @@ func setViperDefaults() { viper.SetDefault("address", "0.0.0.0") viper.SetDefault("port", 4533) viper.SetDefault("unixsocketperm", "0660") + viper.SetDefault("enforcenonrootuser", false) viper.SetDefault("sessiontimeout", consts.DefaultSessionTimeout) viper.SetDefault("baseurl", "") viper.SetDefault("tlscert", "") @@ -717,7 +749,8 @@ func setViperDefaults() { viper.SetDefault("defaultdownsamplingformat", consts.DefaultDownsamplingFormat) viper.SetDefault("search.fullstring", false) viper.SetDefault("search.backend", "fts") - viper.SetDefault("similarsongsmatchthreshold", 85) + viper.SetDefault("matcher.preferstarred", true) + viper.SetDefault("matcher.fuzzythreshold", 85) viper.SetDefault("recentlyaddedbymodtime", false) viper.SetDefault("prefersorttags", false) viper.SetDefault("ignoredarticles", "The El La Los Las Le Les Os As O A") diff --git a/conf/configuration_test.go b/conf/configuration_test.go index 121b1902c..5d4e73fad 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -250,6 +250,49 @@ var _ = Describe("Configuration", func() { ) }) + Describe("EnforceNonRootUser", func() { + It("defaults to false", func() { + conf.Load(true) + + Expect(conf.Server.EnforceNonRootUser).To(BeFalse()) + }) + + It("allows startup for non-root users when enabled", func() { + DeferCleanup(conf.SetRuntimeInfoForTest("linux", 1000)) + viper.Set("enforcenonrootuser", true) + + conf.Load(true) + + Expect(conf.Server.EnforceNonRootUser).To(BeTrue()) + }) + + It("exits when enabled and running as root without having created a data folder", func() { + // Create a path that doesn't exist yet + tempBase := GinkgoT().TempDir() + nonExistentDataFolder := filepath.Join(tempBase, "nonexistent", "data") + DeferCleanup(conf.SetRuntimeInfoForTest("linux", 0)) + viper.Set("enforcenonrootuser", true) + viper.Set("datafolder", nonExistentDataFolder) + + // Attempt to load config as root user - should fail before creating directories + Expect(func() { + conf.Load(true) + }).To(PanicWith(ContainSubstring("EnforceNonRootUser is enabled but Navidrome is running as root"))) + + // Verify that the data folder was NOT created + Expect(nonExistentDataFolder).ToNot(BeAnExistingFile()) + }) + + It("is a no-op on non-unix platforms", func() { + DeferCleanup(conf.SetRuntimeInfoForTest("windows", 0)) + viper.Set("enforcenonrootuser", true) + + conf.Load(true) + + Expect(conf.Server.EnforceNonRootUser).To(BeTrue()) + }) + }) + DescribeTable("should load configuration from", func(format string) { filename := filepath.Join("testdata", "cfg."+format) diff --git a/conf/export_test.go b/conf/export_test.go index 85755aa12..acebca551 100644 --- a/conf/export_test.go +++ b/conf/export_test.go @@ -16,6 +16,17 @@ var ToPascalCase = toPascalCase var ValidateMaxImageUploadSize = validateMaxImageUploadSize +func SetRuntimeInfoForTest(goos string, euid int) func() { + oldGOOS := currentGOOS + oldEUID := getEUID + currentGOOS = func() string { return goos } + getEUID = func() int { return euid } + return func() { + currentGOOS = oldGOOS + getEUID = oldEUID + } +} + func SetLogFatal(f func(...any)) func() { old := logFatal logFatal = f diff --git a/core/artwork/artwork_internal_test.go b/core/artwork/artwork_internal_test.go index 380352d3f..12a7085e8 100644 --- a/core/artwork/artwork_internal_test.go +++ b/core/artwork/artwork_internal_test.go @@ -7,12 +7,11 @@ import ( "image/jpeg" "image/png" "io" - "os" "path/filepath" + "time" _ "github.com/gen2brain/webp" - "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/log" @@ -81,6 +80,7 @@ var _ = Describe("Artwork", func() { }) }) It("returns embed cover", func() { + tests.SkipOnWindows("artwork path handling (#TBD-path-sep-artwork)") aw, err := newAlbumArtworkReader(ctx, aw, alOnlyEmbed.CoverArtID(), nil) Expect(err).ToNot(HaveOccurred()) _, path, err := aw.Reader(ctx) @@ -104,6 +104,7 @@ var _ = Describe("Artwork", func() { }) }) It("returns external cover", func() { + tests.SkipOnWindows("artwork path handling (#TBD-path-sep-artwork)") folderRepo.result = []model.Folder{{ Path: "tests/fixtures/artist/an-album", ImageFiles: []string{"front.png"}, @@ -134,6 +135,7 @@ var _ = Describe("Artwork", func() { }) DescribeTable("CoverArtPriority", func(priority string, expected string) { + tests.SkipOnWindows("artwork path handling (#TBD-path-sep-artwork)") conf.Server.CoverArtPriority = priority aw, err := newAlbumArtworkReader(ctx, aw, alMultipleCovers.CoverArtID(), nil) Expect(err).ToNot(HaveOccurred()) @@ -146,6 +148,51 @@ var _ = Describe("Artwork", func() { Entry(nil, " embedded , front.* , cover.*,folder.*", "tests/fixtures/artist/an-album/test.mp3"), ) }) + Context("LastUpdated", func() { + // Regression test for #5377: LastUpdated feeds the HTTP Last-Modified header. + // It must return max(album.UpdatedAt, ImagesUpdatedAt) so browsers revalidate + // cached cover art when only the image file changes. + now := time.Now().Truncate(time.Second) + DescribeTable("returns the max of album.UpdatedAt and ImagesUpdatedAt", + func(albumUpdatedAt, imagesUpdatedAt, expected time.Time) { + album := model.Album{ID: "al1", UpdatedAt: albumUpdatedAt} + folderRepo.result = []model.Folder{{ImagesUpdatedAt: imagesUpdatedAt}} + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{album}) + + ar, err := newAlbumArtworkReader(ctx, aw, album.CoverArtID(), nil) + Expect(err).ToNot(HaveOccurred()) + Expect(ar.LastUpdated()).To(Equal(expected)) + }, + Entry("album newer than images", now, now.Add(-1*time.Hour), now), + Entry("images newer than album", now.Add(-24*time.Hour), now.Add(-1*time.Hour), now.Add(-1*time.Hour)), + Entry("equal timestamps", now, now, now), + ) + }) + }) + Describe("discArtworkReader", func() { + Context("LastUpdated", func() { + // Regression test for #5377: same bug as albumArtworkReader — disc covers + // must also revalidate when the image file changes, not only when media files do. + now := time.Now().Truncate(time.Second) + DescribeTable("returns the max of album.UpdatedAt and ImagesUpdatedAt", + func(albumUpdatedAt, imagesUpdatedAt, expected time.Time) { + album := model.Album{ID: "al1", UpdatedAt: albumUpdatedAt} + folderRepo.result = []model.Folder{{ImagesUpdatedAt: imagesUpdatedAt}} + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{album}) + ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ + {ID: "mf1", AlbumID: "al1", DiscNumber: 1, Path: "tests/fixtures/test.mp3"}, + }) + + artID := model.NewArtworkID(model.KindDiscArtwork, model.DiscArtworkID("al1", 1), nil) + dr, err := newDiscArtworkReader(ctx, aw, artID) + Expect(err).ToNot(HaveOccurred()) + Expect(dr.LastUpdated()).To(Equal(expected)) + }, + Entry("album newer than images", now, now.Add(-1*time.Hour), now), + Entry("images newer than album", now.Add(-24*time.Hour), now.Add(-1*time.Hour), now.Add(-1*time.Hour)), + Entry("equal timestamps", now, now, now), + ) + }) }) Describe("artistArtworkReader", func() { Context("Multiple covers", func() { @@ -166,6 +213,7 @@ var _ = Describe("Artwork", func() { }) DescribeTable("ArtistArtPriority", func(priority string, expected string) { + tests.SkipOnWindows("artwork path handling (#TBD-path-sep-artwork)") conf.Server.ArtistArtPriority = priority aw, err := newArtistArtworkReader(ctx, aw, arMultipleCovers.CoverArtID(), nil) Expect(err).ToNot(HaveOccurred()) @@ -203,6 +251,7 @@ var _ = Describe("Artwork", func() { }) }) It("returns embed cover", func() { + tests.SkipOnWindows("artwork path handling (#TBD-path-sep-artwork)") aw, err := newMediafileArtworkReader(ctx, aw, mfWithEmbed.CoverArtID()) Expect(err).ToNot(HaveOccurred()) _, path, err := aw.Reader(ctx) @@ -210,6 +259,7 @@ var _ = Describe("Artwork", func() { Expect(path).To(Equal("tests/fixtures/test.mp3")) }) It("returns embed cover if successfully extracted by ffmpeg", func() { + tests.SkipOnWindows("artwork path handling (#TBD-path-sep-artwork)") aw, err := newMediafileArtworkReader(ctx, aw, mfCorruptedCover.CoverArtID()) Expect(err).ToNot(HaveOccurred()) r, path, err := aw.Reader(ctx) diff --git a/core/artwork/reader_album.go b/core/artwork/reader_album.go index 641b12b33..35d489b6c 100644 --- a/core/artwork/reader_album.go +++ b/core/artwork/reader_album.go @@ -72,7 +72,7 @@ func (a *albumArtworkReader) Key() string { ) } func (a *albumArtworkReader) LastUpdated() time.Time { - return a.album.UpdatedAt + return a.lastUpdate } func (a *albumArtworkReader) Reader(ctx context.Context) (io.ReadCloser, string, error) { diff --git a/core/artwork/reader_artist_test.go b/core/artwork/reader_artist_test.go index 5e2066aeb..220c7554f 100644 --- a/core/artwork/reader_artist_test.go +++ b/core/artwork/reader_artist_test.go @@ -12,6 +12,7 @@ import ( "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -61,6 +62,7 @@ var _ = Describe("artistArtworkReader", func() { When("artist has only one album", func() { It("returns the parent folder", func() { + tests.SkipOnWindows("artwork path handling (#TBD-path-sep-artwork)") paths = []string{ filepath.FromSlash("/music/artist/album1"), } @@ -86,6 +88,7 @@ var _ = Describe("artistArtworkReader", func() { When("the album paths contain same prefix", func() { It("returns the common prefix", func() { + tests.SkipOnWindows("artwork path handling (#TBD-path-sep-artwork)") paths = []string{ filepath.FromSlash("/music/artist/album1"), filepath.FromSlash("/music/artist/album2"), diff --git a/core/artwork/reader_disc.go b/core/artwork/reader_disc.go index 5a7a8a65e..30d4968e1 100644 --- a/core/artwork/reader_disc.go +++ b/core/artwork/reader_disc.go @@ -116,7 +116,7 @@ func (d *discArtworkReader) Key() string { } func (d *discArtworkReader) LastUpdated() time.Time { - return d.album.UpdatedAt + return d.lastUpdate } func (d *discArtworkReader) Reader(ctx context.Context) (io.ReadCloser, string, error) { diff --git a/core/common_test.go b/core/common_test.go index c8dde12d9..0d6e3a299 100644 --- a/core/common_test.go +++ b/core/common_test.go @@ -41,6 +41,7 @@ var _ = Describe("common.go", func() { }) It("returns the absolute path when library exists", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-core)") ctx := context.Background() abs := AbsolutePath(ctx, ds, libId, path) Expect(abs).To(Equal("/library/root/music/file.mp3")) diff --git a/core/external/provider_topsongs_test.go b/core/external/provider_topsongs_test.go index 4bd0e5959..0d9b5800d 100644 --- a/core/external/provider_topsongs_test.go +++ b/core/external/provider_topsongs_test.go @@ -30,7 +30,7 @@ var _ = Describe("Provider - TopSongs", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) // Disable fuzzy matching for these tests to avoid unexpected GetAll calls - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 ctx = GinkgoT().Context() diff --git a/core/external/provider_updateartistinfo_test.go b/core/external/provider_updateartistinfo_test.go index e309ece6e..cc9506d1f 100644 --- a/core/external/provider_updateartistinfo_test.go +++ b/core/external/provider_updateartistinfo_test.go @@ -105,6 +105,29 @@ var _ = Describe("Provider - UpdateArtistInfo", func() { ag.AssertExpectations(GinkgoT()) }) + It("preserves decoded plain text in biography storage", func() { + originalArtist := &model.Artist{ + ID: "ar-encoded-bio", + Name: "Encoded Bio Artist", + } + mockArtistRepo.SetData(model.Artists{*originalArtist}) + + expectedMBID := "mbid-encoded-bio" + expectedBio := "R&B" + + ag.On("GetArtistMBID", ctx, "ar-encoded-bio", "Encoded Bio Artist").Return(expectedMBID, nil).Once() + ag.On("GetArtistImages", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID).Return(nil, nil).Maybe() + ag.On("GetArtistBiography", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID).Return(expectedBio, nil).Once() + ag.On("GetArtistURL", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID).Return("", nil).Maybe() + ag.On("GetSimilarArtists", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID, 100).Return(nil, nil).Maybe() + + updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-encoded-bio", 10, false) + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedArtist).NotTo(BeNil()) + Expect(updatedArtist.Biography).To(Equal("R&B")) + }) + It("returns cached info when artist exists and info is not expired", func() { now := time.Now() originalArtist := &model.Artist{ diff --git a/core/lyrics/lyrics_test.go b/core/lyrics/lyrics_test.go index 2e495a714..7e837782e 100644 --- a/core/lyrics/lyrics_test.go +++ b/core/lyrics/lyrics_test.go @@ -10,6 +10,7 @@ import ( "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/lyrics" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" "github.com/navidrome/navidrome/utils" "github.com/navidrome/navidrome/utils/gg" . "github.com/onsi/ginkgo/v2" @@ -93,6 +94,7 @@ var _ = Describe("sources", func() { var accessForbiddenFile string BeforeEach(func() { + tests.SkipOnWindows("uses Unix file permission bits") accessForbiddenFile = utils.TempFileName("access_forbidden-", ".mp3") f, err := os.OpenFile(accessForbiddenFile, os.O_WRONLY|os.O_CREATE, 0222) diff --git a/core/matcher/matcher.go b/core/matcher/matcher.go index 40d4dc160..cf6c99e28 100644 --- a/core/matcher/matcher.go +++ b/core/matcher/matcher.go @@ -46,18 +46,20 @@ func New(ds model.DataStore) *Matcher { // # Fuzzy Matching Details // // For title+artist matching, the algorithm uses Jaro-Winkler similarity (threshold configurable -// via SimilarSongsMatchThreshold, default 85%). Matches are ranked by: +// via Matcher.FuzzyThreshold, default 85%). Matches are ranked by: // // 1. Title similarity (Jaro-Winkler score, 0.0-1.0) // 2. Duration proximity (closer duration = higher score, 1.0 if unknown) -// 3. Specificity level (0-5, based on metadata precision): +// 3. Preferred track flag (enabled by Matcher.PreferStarred; prioritized when the track is +// starred or has rating >= 4) +// 4. Specificity level (0-5, based on metadata precision): // - Level 5: Title + Artist MBID + Album MBID (most specific) // - Level 4: Title + Artist MBID + Album name (fuzzy) // - Level 3: Title + Artist name + Album name (fuzzy) // - Level 2: Title + Artist MBID // - Level 1: Title + Artist name // - Level 0: Title only -// 4. Album similarity (Jaro-Winkler, as final tiebreaker) +// 5. Album similarity (Jaro-Winkler, as final tiebreaker) // // # Examples // @@ -250,6 +252,7 @@ type songQuery struct { type matchScore struct { titleSimilarity float64 durationProximity float64 + preferredMatch bool albumSimilarity float64 specificityLevel int } @@ -262,6 +265,9 @@ func (s matchScore) betterThan(other matchScore) bool { if s.durationProximity != other.durationProximity { return s.durationProximity > other.durationProximity } + if s.preferredMatch != other.preferredMatch { + return s.preferredMatch + } if s.specificityLevel != other.specificityLevel { return s.specificityLevel > other.specificityLevel } @@ -322,7 +328,7 @@ func (m *Matcher) loadTracksByTitleAndArtist(ctx context.Context, songs []agents return map[string]model.MediaFile{}, nil } - threshold := float64(conf.Server.SimilarSongsMatchThreshold) / 100.0 + threshold := float64(conf.Server.Matcher.FuzzyThreshold) / 100.0 byArtist := map[string][]songQuery{} for _, q := range queries { @@ -393,6 +399,7 @@ func (m *Matcher) findBestMatch(q songQuery, sanitizedTracks []sanitizedTrack, t score := matchScore{ titleSimilarity: titleSim, durationProximity: durationProximity(q.durationMs, t.mf.Duration), + preferredMatch: conf.Server.Matcher.PreferStarred && isPreferredTrack(t.mf), albumSimilarity: albumSim, specificityLevel: computeSpecificityLevel(q, t, threshold), } @@ -406,6 +413,10 @@ func (m *Matcher) findBestMatch(q songQuery, sanitizedTracks []sanitizedTrack, t return bestMatch, found } +func isPreferredTrack(mf *model.MediaFile) bool { + return mf.Starred || mf.Rating >= 4 +} + // buildTitleQueries converts agent songs into normalized songQuery structs for title+artist matching. func (m *Matcher) buildTitleQueries(songs []agents.Song, priorMatches ...map[string]model.MediaFile) []songQuery { var queries []songQuery diff --git a/core/matcher/matcher_test.go b/core/matcher/matcher_test.go index b1f59b258..8996cf71d 100644 --- a/core/matcher/matcher_test.go +++ b/core/matcher/matcher_test.go @@ -78,7 +78,7 @@ var _ = Describe("Matcher", func() { Describe("MatchSongsToLibrary", func() { Context("matching by direct ID", func() { It("matches songs with an ID field to MediaFiles by ID", func() { - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ {ID: "track-1", Name: "Some Song", Artist: "Some Artist"}, } @@ -96,7 +96,7 @@ var _ = Describe("Matcher", func() { Context("matching by MBID", func() { It("matches songs with MBID to tracks with matching mbz_recording_id", func() { - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ {Name: "Paranoid Android", MBID: "abc-123", Artist: "Radiohead"}, } @@ -115,7 +115,7 @@ var _ = Describe("Matcher", func() { Context("matching by ISRC", func() { It("matches songs with ISRC to tracks with matching ISRC tag", func() { - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ {Name: "Paranoid Android", ISRC: "GBAYE0000351", Artist: "Radiohead"}, } @@ -134,7 +134,7 @@ var _ = Describe("Matcher", func() { Context("fuzzy title+artist matching", func() { It("matches songs by title and artist name", func() { - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ {Name: "Enjoy the Silence", Artist: "Depeche Mode"}, } @@ -149,7 +149,7 @@ var _ = Describe("Matcher", func() { }) It("matches songs with fuzzy title similarity", func() { - conf.Server.SimilarSongsMatchThreshold = 85 + conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ {Name: "Bohemian Rhapsody", Artist: "Queen"}, } @@ -164,7 +164,7 @@ var _ = Describe("Matcher", func() { }) It("does not match completely different titles", func() { - conf.Server.SimilarSongsMatchThreshold = 85 + conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ {Name: "Yesterday", Artist: "The Beatles"}, } @@ -180,7 +180,7 @@ var _ = Describe("Matcher", func() { Context("deduplication", func() { It("removes duplicates when different input songs match the same library track", func() { - conf.Server.SimilarSongsMatchThreshold = 85 + conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ {Name: "Bohemian Rhapsody (Live)", Artist: "Queen"}, {Name: "Bohemian Rhapsody (Original Mix)", Artist: "Queen"}, @@ -196,7 +196,7 @@ var _ = Describe("Matcher", func() { }) It("preserves duplicates when identical input songs match the same library track", func() { - conf.Server.SimilarSongsMatchThreshold = 85 + conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, @@ -215,7 +215,7 @@ var _ = Describe("Matcher", func() { Context("priority ordering", func() { It("prefers ID match over MBID match", func() { - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 // Song has both ID and MBID set. The matcher should resolve via ID // and short-circuit the MBID phase entirely, so no MBID fetch should // occur even though an mbz_recording_id exists in the input. @@ -236,7 +236,7 @@ var _ = Describe("Matcher", func() { Context("count limit", func() { It("returns at most 'count' results", func() { - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ {Name: "Song A", Artist: "Artist"}, {Name: "Song B", Artist: "Artist"}, @@ -265,7 +265,7 @@ var _ = Describe("Matcher", func() { Describe("specificity level matching", func() { BeforeEach(func() { - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 }) It("matches by title + artist MBID + album MBID (highest priority)", func() { @@ -396,7 +396,7 @@ var _ = Describe("Matcher", func() { Describe("fuzzy matching thresholds", func() { Context("with default threshold (85%)", func() { It("matches songs with remastered suffix", func() { - conf.Server.SimilarSongsMatchThreshold = 85 + conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ {Name: "Paranoid Android", Artist: "Radiohead"}, @@ -415,7 +415,7 @@ var _ = Describe("Matcher", func() { }) It("matches songs with live suffix", func() { - conf.Server.SimilarSongsMatchThreshold = 85 + conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ {Name: "Bohemian Rhapsody", Artist: "Queen"}, @@ -436,7 +436,7 @@ var _ = Describe("Matcher", func() { Context("with threshold set to 100 (exact match only)", func() { It("only matches exact titles", func() { - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ {Name: "Paranoid Android", Artist: "Radiohead"}, @@ -456,7 +456,7 @@ var _ = Describe("Matcher", func() { Context("with lower threshold (75%)", func() { It("matches more aggressively", func() { - conf.Server.SimilarSongsMatchThreshold = 75 + conf.Server.Matcher.FuzzyThreshold = 75 songs := []agents.Song{ {Name: "Song", Artist: "Artist"}, @@ -478,7 +478,8 @@ var _ = Describe("Matcher", func() { Describe("fuzzy album matching", func() { BeforeEach(func() { - conf.Server.SimilarSongsMatchThreshold = 85 + conf.Server.Matcher.FuzzyThreshold = 85 + conf.Server.Matcher.PreferStarred = false }) It("matches album with (Remaster) suffix", func() { @@ -540,11 +541,53 @@ var _ = Describe("Matcher", func() { Expect(result).To(HaveLen(1)) Expect(result[0].ID).To(Equal("exact")) }) + + It("prefers starred songs over better album match when enabled", func() { + conf.Server.Matcher.PreferStarred = true + songs := []agents.Song{ + {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + } + albumMatch := model.MediaFile{ + ID: "album-match", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", + } + starredTrack := model.MediaFile{ + ID: "starred", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Singles", Annotations: model.Annotations{Starred: true}, + } + + setupTitleOnlyExpectations(model.MediaFiles{albumMatch, starredTrack}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("starred")) + }) + + It("prefers 4-star songs over better album match when enabled", func() { + conf.Server.Matcher.PreferStarred = true + songs := []agents.Song{ + {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + } + albumMatch := model.MediaFile{ + ID: "album-match", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", + } + ratedTrack := model.MediaFile{ + ID: "rated", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Singles", Annotations: model.Annotations{Rating: 4}, + } + + setupTitleOnlyExpectations(model.MediaFiles{albumMatch, ratedTrack}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("rated")) + }) }) Describe("duration matching", func() { BeforeEach(func() { - conf.Server.SimilarSongsMatchThreshold = 100 + conf.Server.Matcher.FuzzyThreshold = 100 }) It("prefers tracks with matching duration", func() { @@ -678,7 +721,7 @@ var _ = Describe("Matcher", func() { Describe("deduplication edge cases", func() { BeforeEach(func() { - conf.Server.SimilarSongsMatchThreshold = 85 + conf.Server.Matcher.FuzzyThreshold = 85 }) It("handles mixed scenario with both identical and different input songs", func() { diff --git a/core/playback/mpv/mpv_test.go b/core/playback/mpv/mpv_test.go index b1f2435a3..6754b39ac 100644 --- a/core/playback/mpv/mpv_test.go +++ b/core/playback/mpv/mpv_test.go @@ -14,6 +14,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -199,6 +200,7 @@ var _ = Describe("MPV", func() { }) It("executes MPV command and captures arguments correctly", func() { + tests.SkipOnWindows("mpv binary not available in CI (#TBD-mpv-windows)") ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() @@ -226,6 +228,7 @@ var _ = Describe("MPV", func() { }) It("handles file paths with spaces", func() { + tests.SkipOnWindows("mpv binary not available in CI (#TBD-mpv-windows)") ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() @@ -253,6 +256,7 @@ var _ = Describe("MPV", func() { }) It("passes all snapcast arguments correctly", func() { + tests.SkipOnWindows("mpv binary not available in CI (#TBD-mpv-windows)") ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() diff --git a/core/playlists/import_test.go b/core/playlists/import_test.go index a6320bc7e..53855d781 100644 --- a/core/playlists/import_test.go +++ b/core/playlists/import_test.go @@ -183,6 +183,7 @@ var _ = Describe("Playlists - Import", func() { }) It("rejects #EXTALBUMARTURL with absolute path outside library boundaries", func() { + tests.SkipOnWindows("relies on Unix /etc filesystem") tmpDir := GinkgoT().TempDir() m3u := "#EXTALBUMARTURL:/etc/passwd\ntest.mp3\n" @@ -320,6 +321,7 @@ var _ = Describe("Playlists - Import", func() { Expect(pls.Rules.Expression).To(BeAssignableToTypeOf(criteria.All{})) }) It("returns an error if the playlist is not well-formed", func() { + tests.SkipOnWindows("line-ending differences affect JSON error offset") _, err := ps.ImportFile(ctx, folder, "invalid_json.nsp") Expect(err.Error()).To(ContainSubstring("line 19, column 1: invalid character '\\n'")) }) @@ -347,6 +349,7 @@ var _ = Describe("Playlists - Import", func() { DescribeTable("Playlist filename Unicode normalization (regression fix-playlist-filename-normalization)", func(storedForm, filesystemForm string) { + tests.SkipOnWindows("/tmp hardcoded in test") // Use Polish characters that decompose: ó (U+00F3) -> o + combining acute (U+006F + U+0301) plsNameNFC := "Piosenki_Polskie_zółć" // NFC form (composed) plsNameNFD := norm.NFD.String(plsNameNFC) @@ -821,6 +824,7 @@ var _ = Describe("Playlists - Import", func() { }) It("returns true if folder is in PlaylistsPath", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-playlists)") conf.Server.PlaylistsPath = "other/**:playlists/**" Expect(playlists.InPath(folder)).To(BeTrue()) }) diff --git a/core/playlists/parse_m3u_test.go b/core/playlists/parse_m3u_test.go index 05e1c30e1..d7fd5e001 100644 --- a/core/playlists/parse_m3u_test.go +++ b/core/playlists/parse_m3u_test.go @@ -15,6 +15,7 @@ var _ = Describe("libraryMatcher", func() { ctx := context.Background() BeforeEach(func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-playlists)") mockLibRepo = &tests.MockLibraryRepo{} ds = &tests.MockDataStore{ MockedLibrary: mockLibRepo, @@ -196,6 +197,7 @@ var _ = Describe("pathResolver", func() { ctx := context.Background() BeforeEach(func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-playlists)") mockLibRepo = &tests.MockLibraryRepo{} ds = &tests.MockDataStore{ MockedLibrary: mockLibRepo, diff --git a/core/storage/local/local_test.go b/core/storage/local/local_test.go index b977ef4a5..aef89cdd5 100644 --- a/core/storage/local/local_test.go +++ b/core/storage/local/local_test.go @@ -13,6 +13,7 @@ import ( "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core/storage" "github.com/navidrome/navidrome/model/metadata" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -44,6 +45,10 @@ var _ = Describe("LocalStorage", func() { }) Describe("newLocalStorage", func() { + BeforeEach(func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage-local)") + }) + Context("with valid path", func() { It("should create a localStorage instance with correct path", func() { u, err := url.Parse("file://" + tempDir) @@ -166,6 +171,10 @@ var _ = Describe("LocalStorage", func() { }) Describe("localStorage.FS", func() { + BeforeEach(func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage-local)") + }) + Context("with existing directory", func() { It("should return a localFS instance", func() { u, err := url.Parse("file://" + tempDir) @@ -199,6 +208,7 @@ var _ = Describe("LocalStorage", func() { var testFile string BeforeEach(func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage-local)") // Create a test file testFile = filepath.Join(tempDir, "test.mp3") err := os.WriteFile(testFile, []byte("test data"), 0600) @@ -380,6 +390,7 @@ var _ = Describe("LocalStorage", func() { Describe("Storage registration", func() { It("should register localStorage for file scheme", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage-local)") // This tests the init() function indirectly storage, err := storage.For("file://" + tempDir) Expect(err).ToNot(HaveOccurred()) diff --git a/core/storage/storage_test.go b/core/storage/storage_test.go index 60496e611..32fbac413 100644 --- a/core/storage/storage_test.go +++ b/core/storage/storage_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "testing" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -54,6 +55,7 @@ var _ = Describe("Storage", func() { Expect(s.(*fakeLocalStorage).u.Path).To(Equal("/tmp")) }) It("should return a file implementation for a relative folder", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage)") s, err := For("tmp") Expect(err).ToNot(HaveOccurred()) cwd, _ := os.Getwd() diff --git a/model/folder_test.go b/model/folder_test.go index c136dfa96..221cbf611 100644 --- a/model/folder_test.go +++ b/model/folder_test.go @@ -7,6 +7,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/id" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -92,6 +93,7 @@ var _ = Describe("Folder", func() { When("the folder has multiple subdirs", func() { It("should return the correct folder ID", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") folderPath := filepath.FromSlash("/music/rock/metal") expectedID := id.NewHash("1:rock/metal") Expect(model.FolderID(lib, folderPath)).To(Equal(expectedID)) @@ -101,6 +103,7 @@ var _ = Describe("Folder", func() { Describe("NewFolder", func() { It("should create a new SubFolder with the correct attributes", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") folderPath := filepath.FromSlash("rock/metal") folder := model.NewFolder(lib, folderPath) diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 8b0c13da2..3547ec4ef 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -6,6 +6,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" . "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -22,7 +23,7 @@ var _ = Describe("MediaFiles", func() { SortAlbumName: "SortAlbumName", SortArtistName: "SortArtistName", SortAlbumArtistName: "SortAlbumArtistName", OrderAlbumName: "OrderAlbumName", OrderAlbumArtistName: "OrderAlbumArtistName", MbzAlbumArtistID: "MbzAlbumArtistID", MbzAlbumType: "MbzAlbumType", MbzAlbumComment: "MbzAlbumComment", - MbzReleaseGroupID: "MbzReleaseGroupID", Compilation: false, CatalogNum: "", Path: "/music1/file1.mp3", FolderID: "Folder1", + MbzReleaseGroupID: "MbzReleaseGroupID", Compilation: false, CatalogNum: "", Path: "music1/file1.mp3", FolderID: "Folder1", }, { ID: "2", Album: "Album", ArtistID: "ArtistID", Artist: "Artist", AlbumArtistID: "AlbumArtistID", AlbumArtist: "AlbumArtist", AlbumID: "AlbumID", @@ -30,7 +31,7 @@ var _ = Describe("MediaFiles", func() { OrderAlbumName: "OrderAlbumName", OrderArtistName: "OrderArtistName", OrderAlbumArtistName: "OrderAlbumArtistName", MbzAlbumArtistID: "MbzAlbumArtistID", MbzAlbumType: "MbzAlbumType", MbzAlbumComment: "MbzAlbumComment", MbzReleaseGroupID: "MbzReleaseGroupID", - Compilation: true, CatalogNum: "CatalogNum", HasCoverArt: true, Path: "/music2/file2.mp3", FolderID: "Folder2", + Compilation: true, CatalogNum: "CatalogNum", HasCoverArt: true, Path: "music2/file2.mp3", FolderID: "Folder2", }, } }) @@ -51,7 +52,7 @@ var _ = Describe("MediaFiles", func() { Expect(album.MbzReleaseGroupID).To(Equal("MbzReleaseGroupID")) Expect(album.CatalogNum).To(Equal("CatalogNum")) Expect(album.Compilation).To(BeTrue()) - Expect(album.EmbedArtPath).To(Equal("/music2/file2.mp3")) + Expect(album.EmbedArtPath).To(Equal("music2/file2.mp3")) Expect(album.FolderIDs).To(ConsistOf("Folder1", "Folder2")) }) }) @@ -447,6 +448,9 @@ var _ = Describe("MediaFiles", func() { DescribeTable("generates correct output", func(absolutePaths bool, expectedContent string) { + if absolutePaths { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") + } result := mfs.ToM3U8("Multi Track", absolutePaths) Expect(result).To(Equal(expectedContent)) }, @@ -467,6 +471,7 @@ var _ = Describe("MediaFiles", func() { Context("path variations", func() { It("handles different path structures", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") mfs = MediaFiles{ {Title: "Root", Artist: "Artist", Duration: 60, Path: "song.mp3", LibraryPath: "/lib"}, {Title: "Nested", Artist: "Artist", Duration: 60, Path: "deep/nested/song.mp3", LibraryPath: "/lib"}, diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index 47f5ca63f..eb66d11d1 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -6,6 +6,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -79,6 +80,7 @@ var _ = Describe("getPID", func() { }) When("field is folder", func() { It("should return the pid", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-metadata)") spec := "folder|title" md.tags = map[model.TagName][]string{"title": {"title"}} mf.Path = "/path/to/file.mp3" diff --git a/model/playlist_test.go b/model/playlist_test.go index a54cecd53..9ed24f00f 100644 --- a/model/playlist_test.go +++ b/model/playlist_test.go @@ -2,6 +2,7 @@ package model_test import ( "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -27,6 +28,7 @@ var _ = Describe("Playlist", func() { } }) It("generates the correct M3U format", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") expected := `#EXTM3U #PLAYLIST:Mellow sunset #EXTINF:378,Morcheeba feat. Kurt Wagner - What New York Couples Fight About diff --git a/persistence/folder_repository_test.go b/persistence/folder_repository_test.go index 7b6a0f764..ebc08fd04 100644 --- a/persistence/folder_repository_test.go +++ b/persistence/folder_repository_test.go @@ -8,6 +8,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/pocketbase/dbx" @@ -99,6 +100,7 @@ var _ = Describe("FolderRepository", func() { }) It("includes all child folders when querying parent", func() { + tests.SkipOnWindows("path storage (#TBD-path-sep-persistence)") // Create a parent folder with multiple children parent := model.NewFolder(testLib, "TestParent/Music") child1 := model.NewFolder(testLib, "TestParent/Music/Rock/Queen") @@ -120,6 +122,7 @@ var _ = Describe("FolderRepository", func() { }) It("excludes children from other libraries", func() { + tests.SkipOnWindows("path storage (#TBD-path-sep-persistence)") // Create parent in testLib parent := model.NewFolder(testLib, "TestIsolation/Parent") child := model.NewFolder(testLib, "TestIsolation/Parent/Child") @@ -145,6 +148,7 @@ var _ = Describe("FolderRepository", func() { }) It("excludes missing children when querying parent", func() { + tests.SkipOnWindows("path storage (#TBD-path-sep-persistence)") // Create parent and children, mark one as missing parent := model.NewFolder(testLib, "TestMissingChild/Parent") child1 := model.NewFolder(testLib, "TestMissingChild/Parent/Child1") @@ -165,6 +169,7 @@ var _ = Describe("FolderRepository", func() { }) It("handles mix of existing and non-existing target paths", func() { + tests.SkipOnWindows("path storage (#TBD-path-sep-persistence)") // Create folders for one path but not the other existingParent := model.NewFolder(testLib, "TestMixed/Exists") existingChild := model.NewFolder(testLib, "TestMixed/Exists/Child") diff --git a/persistence/library_repository_test.go b/persistence/library_repository_test.go index 3e3972bdb..de7161643 100644 --- a/persistence/library_repository_test.go +++ b/persistence/library_repository_test.go @@ -2,6 +2,7 @@ package persistence import ( "context" + "time" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -64,6 +65,11 @@ var _ = Describe("LibraryRepository", func() { originalID := lib.ID originalCreatedAt := lib.CreatedAt + // Ensure the update's timestamp is strictly greater than the + // create's timestamp on platforms with coarse clock resolution + // (Windows' time.Now() is millisecond-granular). + time.Sleep(2 * time.Millisecond) + // Now update it lib.Name = "Updated Library" lib.Path = "/music/updated" diff --git a/persistence/mediafile_repository_test.go b/persistence/mediafile_repository_test.go index 5a866379f..464d88288 100644 --- a/persistence/mediafile_repository_test.go +++ b/persistence/mediafile_repository_test.go @@ -48,10 +48,10 @@ var _ = Describe("MediaRepository", func() { var mp3File, flacFile1, flacFile2, flacUpperFile model.MediaFile BeforeEach(func() { - mp3File = model.MediaFile{ID: "suffix-mp3", LibraryID: 1, Suffix: "mp3", Path: "/test/file.mp3"} - flacFile1 = model.MediaFile{ID: "suffix-flac1", LibraryID: 1, Suffix: "flac", Path: "/test/file1.flac"} - flacFile2 = model.MediaFile{ID: "suffix-flac2", LibraryID: 1, Suffix: "flac", Path: "/test/file2.flac"} - flacUpperFile = model.MediaFile{ID: "suffix-FLAC", LibraryID: 1, Suffix: "FLAC", Path: "/test/file.FLAC"} + mp3File = model.MediaFile{ID: "suffix-mp3", LibraryID: 1, Suffix: "mp3", Path: "test/file.mp3"} + flacFile1 = model.MediaFile{ID: "suffix-flac1", LibraryID: 1, Suffix: "flac", Path: "test/file1.flac"} + flacFile2 = model.MediaFile{ID: "suffix-flac2", LibraryID: 1, Suffix: "flac", Path: "test/file2.flac"} + flacUpperFile = model.MediaFile{ID: "suffix-FLAC", LibraryID: 1, Suffix: "FLAC", Path: "test/file.FLAC"} Expect(mr.Put(&mp3File)).To(Succeed()) Expect(mr.Put(&flacFile1)).To(Succeed()) @@ -109,7 +109,7 @@ var _ = Describe("MediaRepository", func() { Describe("Put CreatedAt behavior (#5050)", func() { It("sets CreatedAt to now when inserting a new file with zero CreatedAt", func() { before := time.Now().Add(-time.Second) - newFile := model.MediaFile{ID: id.NewRandom(), LibraryID: 1, Path: "/test/created-at-zero.mp3"} + newFile := model.MediaFile{ID: id.NewRandom(), LibraryID: 1, Path: "test/created-at-zero.mp3"} Expect(mr.Put(&newFile)).To(Succeed()) retrieved, err := mr.Get(newFile.ID) @@ -124,7 +124,7 @@ var _ = Describe("MediaRepository", func() { newFile := model.MediaFile{ ID: id.NewRandom(), LibraryID: 1, - Path: "/test/created-at-preserved.mp3", + Path: "test/created-at-preserved.mp3", CreatedAt: originalTime, } Expect(mr.Put(&newFile)).To(Succeed()) @@ -142,7 +142,7 @@ var _ = Describe("MediaRepository", func() { newFile := model.MediaFile{ ID: fileID, LibraryID: 1, - Path: "/test/created-at-update.mp3", + Path: "test/created-at-update.mp3", Title: "Original Title", CreatedAt: originalTime, } @@ -152,7 +152,7 @@ var _ = Describe("MediaRepository", func() { updatedFile := model.MediaFile{ ID: fileID, LibraryID: 1, - Path: "/test/created-at-update.mp3", + Path: "test/created-at-update.mp3", Title: "Updated Title", // CreatedAt is zero - should NOT overwrite the stored value } @@ -231,7 +231,7 @@ var _ = Describe("MediaRepository", func() { It("returns 0 when no ratings exist", func() { newID := id.NewRandom() - Expect(mr.Put(&model.MediaFile{LibraryID: 1, ID: newID, Path: "/test/no-rating.mp3"})).To(Succeed()) + Expect(mr.Put(&model.MediaFile{LibraryID: 1, ID: newID, Path: "test/no-rating.mp3"})).To(Succeed()) mf, err := mr.Get(newID) Expect(err).ToNot(HaveOccurred()) @@ -242,7 +242,7 @@ var _ = Describe("MediaRepository", func() { It("returns the user's rating as average when only one user rated", func() { newID := id.NewRandom() - Expect(mr.Put(&model.MediaFile{LibraryID: 1, ID: newID, Path: "/test/single-rating.mp3"})).To(Succeed()) + Expect(mr.Put(&model.MediaFile{LibraryID: 1, ID: newID, Path: "test/single-rating.mp3"})).To(Succeed()) Expect(mr.SetRating(5, newID)).To(Succeed()) mf, err := mr.Get(newID) @@ -255,7 +255,7 @@ var _ = Describe("MediaRepository", func() { It("calculates average across multiple users", func() { newID := id.NewRandom() - Expect(mr.Put(&model.MediaFile{LibraryID: 1, ID: newID, Path: "/test/multi-rating.mp3"})).To(Succeed()) + Expect(mr.Put(&model.MediaFile{LibraryID: 1, ID: newID, Path: "test/multi-rating.mp3"})).To(Succeed()) Expect(mr.SetRating(3, newID)).To(Succeed()) @@ -273,7 +273,7 @@ var _ = Describe("MediaRepository", func() { It("excludes zero ratings from average calculation", func() { newID := id.NewRandom() - Expect(mr.Put(&model.MediaFile{LibraryID: 1, ID: newID, Path: "/test/zero-excluded.mp3"})).To(Succeed()) + Expect(mr.Put(&model.MediaFile{LibraryID: 1, ID: newID, Path: "test/zero-excluded.mp3"})).To(Succeed()) Expect(mr.SetRating(4, newID)).To(Succeed()) @@ -343,19 +343,19 @@ var _ = Describe("MediaRepository", func() { ID: id.NewRandom(), LibraryID: 1, Title: "Old Song", - Path: "/test/old.mp3", + Path: "test/old.mp3", }, { ID: id.NewRandom(), LibraryID: 1, Title: "Middle Song", - Path: "/test/middle.mp3", + Path: "test/middle.mp3", }, { ID: id.NewRandom(), LibraryID: 1, Title: "New Song", - Path: "/test/new.mp3", + Path: "test/new.mp3", }, } @@ -486,7 +486,7 @@ var _ = Describe("MediaRepository", func() { var mfWithoutAnnotation model.MediaFile BeforeEach(func() { - mfWithoutAnnotation = model.MediaFile{ID: "no-annotation-file", LibraryID: 1, Path: "/test/no-annotation.mp3", Title: "No Annotation"} + mfWithoutAnnotation = model.MediaFile{ID: "no-annotation-file", LibraryID: 1, Path: "test/no-annotation.mp3", Title: "No Annotation"} Expect(mr.Put(&mfWithoutAnnotation)).To(Succeed()) }) @@ -566,7 +566,7 @@ var _ = Describe("MediaRepository", func() { MbzRecordingID: "550e8400-e29b-41d4-a716-446655440020", // Valid UUID v4 MbzReleaseTrackID: "550e8400-e29b-41d4-a716-446655440021", // Valid UUID v4 LibraryID: 1, - Path: "/test/path/test.mp3", + Path: "test/path/test.mp3", } // Insert the test media file into the database @@ -608,7 +608,7 @@ var _ = Describe("MediaRepository", func() { Title: "Test Missing MBID MediaFile", MbzRecordingID: "550e8400-e29b-41d4-a716-446655440022", LibraryID: 1, - Path: "/test/path/missing.mp3", + Path: "test/path/missing.mp3", Missing: true, } diff --git a/persistence/persistence_suite_test.go b/persistence/persistence_suite_test.go index 3ed443129..ebc247d77 100644 --- a/persistence/persistence_suite_test.go +++ b/persistence/persistence_suite_test.go @@ -77,14 +77,14 @@ var ( ) 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}) - albumCJK = al(model.Album{ID: "105", Name: "COWBOY BEBOP", AlbumArtist: "シートベルツ", OrderAlbumName: "cowboy bebop", AlbumArtistID: "4", EmbedArtPath: p("/seatbelts/cowboy-bebop/track1.mp3"), SongCount: 1}) - albumWithVersion = alWithTags(model.Album{ID: "106", Name: "Abbey Road", AlbumArtist: "The Beatles", OrderAlbumName: "abbey road", AlbumArtistID: "3", EmbedArtPath: p("/beatles/2/come together.mp3"), SongCount: 1, MaxYear: 2019}, + 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}) + albumCJK = al(model.Album{ID: "105", Name: "COWBOY BEBOP", AlbumArtist: "シートベルツ", OrderAlbumName: "cowboy bebop", AlbumArtistID: "4", EmbedArtPath: p("seatbelts/cowboy-bebop/track1.mp3"), SongCount: 1}) + albumWithVersion = alWithTags(model.Album{ID: "106", Name: "Abbey Road", AlbumArtist: "The Beatles", OrderAlbumName: "abbey road", AlbumArtistID: "3", EmbedArtPath: p("beatles/2/come together.mp3"), SongCount: 1, MaxYear: 2019}, model.Tags{model.TagAlbumVersion: {"Deluxe Edition"}}) - albumPunctuation = al(model.Album{ID: "107", Name: "Things Fall Apart", AlbumArtist: "The Roots", OrderAlbumName: "things fall apart", AlbumArtistID: "5", EmbedArtPath: p("/roots/things/track1.mp3"), SongCount: 1}) + albumPunctuation = al(model.Album{ID: "107", Name: "Things Fall Apart", AlbumArtist: "The Roots", OrderAlbumName: "things fall apart", AlbumArtistID: "5", EmbedArtPath: p("roots/things/track1.mp3"), SongCount: 1}) testAlbums = model.Albums{ albumSgtPeppers, albumAbbeyRoad, @@ -97,12 +97,12 @@ var ( ) var ( - songDayInALife = mf(model.MediaFile{ID: "1001", Title: "A Day In A Life", ArtistID: "3", Artist: "The Beatles", AlbumID: "101", Album: "Sgt Peppers", Path: p("/beatles/1/sgt/a day.mp3")}) - songComeTogether = mf(model.MediaFile{ID: "1002", Title: "Come Together", ArtistID: "3", Artist: "The Beatles", AlbumID: "102", Album: "Abbey Road", Path: p("/beatles/1/come together.mp3")}) - songRadioactivity = mf(model.MediaFile{ID: "1003", Title: "Radioactivity", ArtistID: "2", Artist: "Kraftwerk", AlbumID: "103", Album: "Radioactivity", Path: p("/kraft/radio/radio.mp3")}) + songDayInALife = mf(model.MediaFile{ID: "1001", Title: "A Day In A Life", ArtistID: "3", Artist: "The Beatles", AlbumID: "101", Album: "Sgt Peppers", Path: p("beatles/1/sgt/a day.mp3")}) + songComeTogether = mf(model.MediaFile{ID: "1002", Title: "Come Together", ArtistID: "3", Artist: "The Beatles", AlbumID: "102", Album: "Abbey Road", Path: p("beatles/1/come together.mp3")}) + songRadioactivity = mf(model.MediaFile{ID: "1003", Title: "Radioactivity", ArtistID: "2", Artist: "Kraftwerk", AlbumID: "103", Album: "Radioactivity", Path: p("kraft/radio/radio.mp3")}) songAntenna = mf(model.MediaFile{ID: "1004", Title: "Antenna", ArtistID: "2", Artist: "Kraftwerk", AlbumID: "103", - Path: p("/kraft/radio/antenna.mp3"), + Path: p("kraft/radio/antenna.mp3"), RGAlbumGain: gg.P(1.0), RGAlbumPeak: gg.P(2.0), RGTrackGain: gg.P(3.0), RGTrackPeak: gg.P(4.0), }) songAntennaWithLyrics = mf(model.MediaFile{ @@ -115,13 +115,13 @@ var ( }) songAntenna2 = mf(model.MediaFile{ID: "1006", Title: "Antenna", ArtistID: "2", Artist: "Kraftwerk", AlbumID: "103"}) // 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"}) - songCJK = mf(model.MediaFile{ID: "3001", Title: "プラチナ・ジェット", ArtistID: "4", Artist: "シートベルツ", AlbumID: "105", Album: "COWBOY BEBOP", Path: p("/seatbelts/cowboy-bebop/track1.mp3")}) - songVersioned = mf(model.MediaFile{ID: "3002", Title: "Come Together", ArtistID: "3", Artist: "The Beatles", AlbumID: "106", Album: "Abbey Road", Path: p("/beatles/2/come together.mp3")}) - songPunctuation = mf(model.MediaFile{ID: "3003", Title: "!!!!!!!", ArtistID: "5", Artist: "The Roots", AlbumID: "107", Album: "Things Fall Apart", Path: p("/roots/things/track1.mp3")}) + 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"}) + songCJK = mf(model.MediaFile{ID: "3001", Title: "プラチナ・ジェット", ArtistID: "4", Artist: "シートベルツ", AlbumID: "105", Album: "COWBOY BEBOP", Path: p("seatbelts/cowboy-bebop/track1.mp3")}) + songVersioned = mf(model.MediaFile{ID: "3002", Title: "Come Together", ArtistID: "3", Artist: "The Beatles", AlbumID: "106", Album: "Abbey Road", Path: p("beatles/2/come together.mp3")}) + songPunctuation = mf(model.MediaFile{ID: "3003", Title: "!!!!!!!", ArtistID: "5", Artist: "The Roots", AlbumID: "107", Album: "Things Fall Apart", Path: p("roots/things/track1.mp3")}) testSongs = model.MediaFiles{ songDayInALife, songComeTogether, diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go index c091cb32b..88cb5f697 100644 --- a/persistence/playlist_repository_test.go +++ b/persistence/playlist_repository_test.go @@ -408,7 +408,7 @@ var _ = Describe("PlaylistRepository", func() { ArtistID: "1", Album: "Test Album", AlbumID: "101", - Path: "/test/grouping/song1.mp3", + Path: "test/grouping/song1.mp3", Tags: model.Tags{ "grouping": []string{"My Crate"}, }, @@ -426,7 +426,7 @@ var _ = Describe("PlaylistRepository", func() { ArtistID: "1", Album: "Test Album", AlbumID: "101", - Path: "/test/grouping/song2.mp3", + Path: "test/grouping/song2.mp3", Tags: model.Tags{}, Participants: model.Participants{}, LibraryID: 1, @@ -614,7 +614,7 @@ var _ = Describe("PlaylistRepository", func() { ArtistID: "1", Album: "Test Album", AlbumID: "101", - Path: "/music/lib1/song.mp3", + Path: "lib1/song.mp3", LibraryID: 1, Participants: model.Participants{}, Tags: model.Tags{}, @@ -630,7 +630,7 @@ var _ = Describe("PlaylistRepository", func() { ArtistID: "1", Album: "Test Album", AlbumID: "101", - Path: uniqueLibPath + "/song.mp3", + Path: "lib2/song.mp3", LibraryID: lib2ID, Participants: model.Participants{}, Tags: model.Tags{}, diff --git a/plugins/config_validation_test.go b/plugins/config_validation_test.go index 20e1ce29b..b430c0b31 100644 --- a/plugins/config_validation_test.go +++ b/plugins/config_validation_test.go @@ -1,5 +1,3 @@ -//go:build !windows - package plugins import ( diff --git a/plugins/manager_loader_test.go b/plugins/manager_loader_test.go index 3a00b07b7..cc07f0611 100644 --- a/plugins/manager_loader_test.go +++ b/plugins/manager_loader_test.go @@ -1,5 +1,3 @@ -//go:build !windows - package plugins import ( diff --git a/plugins/manager_test.go b/plugins/manager_test.go index 6cf90994a..9b6f7ea39 100644 --- a/plugins/manager_test.go +++ b/plugins/manager_test.go @@ -1,3 +1,5 @@ +//go:build !windows + package plugins import ( diff --git a/plugins/manager_watcher_test.go b/plugins/manager_watcher_test.go index 99326bde1..5b5ffca02 100644 --- a/plugins/manager_watcher_test.go +++ b/plugins/manager_watcher_test.go @@ -1,3 +1,5 @@ +//go:build !windows + package plugins import ( diff --git a/plugins/metadata_agent_test.go b/plugins/metadata_agent_test.go index 694cef716..067ae80ca 100644 --- a/plugins/metadata_agent_test.go +++ b/plugins/metadata_agent_test.go @@ -1,3 +1,5 @@ +//go:build !windows + package plugins import ( diff --git a/plugins/migrate_test.go b/plugins/migrate_test.go index 17ed43c5c..568ad34cb 100644 --- a/plugins/migrate_test.go +++ b/plugins/migrate_test.go @@ -1,5 +1,3 @@ -//go:build !windows - package plugins import ( diff --git a/plugins/plugins_suite_windows_test.go b/plugins/plugins_suite_windows_test.go new file mode 100644 index 000000000..ed43bdcc3 --- /dev/null +++ b/plugins/plugins_suite_windows_test.go @@ -0,0 +1,23 @@ +//go:build windows + +package plugins + +import ( + "testing" + + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// Runs the subset of plugin specs compiled on Windows (files without the +// //go:build !windows tag): capabilities, manager_cache, manager_plugin, +// manifest, package. WASM-runtime-dependent specs live in !windows-tagged +// files and aren't reached here. +func TestPlugins(t *testing.T) { + tests.Init(t, false) + log.SetLevel(log.LevelFatal) + RegisterFailHandler(Fail) + RunSpecs(t, "Plugins Suite") +} diff --git a/scanner/phase_4_playlists_test.go b/scanner/phase_4_playlists_test.go index 0b50d39cb..06e6fa686 100644 --- a/scanner/phase_4_playlists_test.go +++ b/scanner/phase_4_playlists_test.go @@ -111,6 +111,7 @@ var _ = Describe("phasePlaylists", func() { }) It("reports an error if there is an error reading files", func() { + tests.SkipOnWindows("relies on Unix /etc filesystem") progress := make(chan *ProgressInfo) state.progress = progress folder := &model.Folder{Path: "/invalid/path"} diff --git a/scanner/scanner_multilibrary_test.go b/scanner/scanner_multilibrary_test.go index 856015239..3ae50933c 100644 --- a/scanner/scanner_multilibrary_test.go +++ b/scanner/scanner_multilibrary_test.go @@ -43,6 +43,7 @@ var _ = Describe("Scanner - Multi-Library", Ordered, func() { } BeforeAll(func() { + tests.SkipOnWindows("SQLite file lock blocks TempDir cleanup (#TBD-path-sep-scanner)") ctx = request.WithUser(GinkgoT().Context(), model.User{ID: "123", IsAdmin: true}) tmpDir := GinkgoT().TempDir() conf.Server.DbPath = filepath.Join(tmpDir, "test-scanner-multilibrary.db?_journal_mode=WAL") diff --git a/scanner/scanner_selective_test.go b/scanner/scanner_selective_test.go index 594b74e38..6c70eb268 100644 --- a/scanner/scanner_selective_test.go +++ b/scanner/scanner_selective_test.go @@ -34,6 +34,7 @@ var _ = Describe("ScanFolders", Ordered, func() { var fsys storagetest.FakeFS BeforeAll(func() { + tests.SkipOnWindows("SQLite file lock blocks TempDir cleanup (#TBD-path-sep-scanner)") ctx = request.WithUser(GinkgoT().Context(), model.User{ID: "123", IsAdmin: true}) tmpDir := GinkgoT().TempDir() conf.Server.DbPath = filepath.Join(tmpDir, "test-selective-scan.db?_journal_mode=WAL") diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index 922d21e62..7bf91d64f 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -168,6 +168,7 @@ var _ = Describe("Scanner", Ordered, func() { }) It("should update the album", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") Expect(runScanner(ctx, true)).To(Succeed()) albums, err := ds.Album(ctx).GetAll(model.QueryOptions{Filters: squirrel.Eq{"album.name": "Help!"}}) @@ -268,6 +269,7 @@ var _ = Describe("Scanner", Ordered, func() { var beatlesMBID = uuid.NewString() BeforeEach(func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") By("Having two MP3 albums") beatles := _t{ "artist": "The Beatles", @@ -872,6 +874,7 @@ var _ = Describe("Scanner", Ordered, func() { }) It("should update artist stats during quick scans when new albums are added", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") // Don't use the mocked artist repo for this test - we need the real one ds.MockedArtist = nil diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index c9add0bd1..42b7af7ba 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -12,6 +12,7 @@ import ( "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/storage" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "golang.org/x/sync/errgroup" @@ -229,6 +230,7 @@ var _ = Describe("walk_dir_tree", func() { Context("with symlinks enabled", func() { BeforeEach(func() { + tests.SkipOnWindows("symlink semantics") conf.Server.Scanner.FollowSymlinks = true }) diff --git a/scanner/watcher_test.go b/scanner/watcher_test.go index e1600db32..a4016d470 100644 --- a/scanner/watcher_test.go +++ b/scanner/watcher_test.go @@ -389,6 +389,7 @@ var _ = Describe("Watcher", func() { }) It("should NOT send notification when nested ignored folder is deleted", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") startEventProcessing() // Simulate deletion of music/rock/artist/temp (matches **/temp) @@ -402,6 +403,7 @@ var _ = Describe("Watcher", func() { }) It("should send notification for non-ignored nested folder", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") startEventProcessing() // Simulate change in music/rock/artist (doesn't match any pattern) @@ -426,6 +428,7 @@ var _ = Describe("Watcher", func() { }) It("should NOT send notification for file changes in ignored folders", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") startEventProcessing() // Simulate file change in rock/_TEMP/file.mp3 @@ -464,11 +467,13 @@ var _ = Describe("resolveFolderPath", func() { }) It("walks up to parent directory when given a file path", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") result := resolveFolderPath(mockFS, "artist1/album1/track1.mp3") Expect(result).To(Equal("artist1/album1")) }) It("walks up multiple levels if needed", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") result := resolveFolderPath(mockFS, "artist1/album1/nonexistent/file.mp3") Expect(result).To(Equal("artist1/album1")) }) @@ -489,6 +494,7 @@ var _ = Describe("resolveFolderPath", func() { }) It("handles nested file paths correctly", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") result := resolveFolderPath(mockFS, "artist1/album2/song.flac") Expect(result).To(Equal("artist1/album2")) }) diff --git a/server/e2e/e2e_suite_test.go b/server/e2e/e2e_suite_test.go index 5b3500f7a..4ad9e3daa 100644 --- a/server/e2e/e2e_suite_test.go +++ b/server/e2e/e2e_suite_test.go @@ -470,6 +470,13 @@ var _ = BeforeSuite(func() { Expect(os.WriteFile(snapshotPath, data, 0600)).To(Succeed()) }) +// Close the database before the suite's TempDir cleanup runs. Required on +// Windows where open SQLite handles hold file locks that block temp-dir +// removal; harmless on other OSes. +var _ = AfterSuite(func() { + db.Close(ctx) +}) + // setupTestDB restores the database from the golden snapshot and creates the // Subsonic Router. Call this from BeforeEach/BeforeAll in each test container. func setupTestDB() { diff --git a/server/nativeapi/translations_test.go b/server/nativeapi/translations_test.go index 06ad7addf..6c834070c 100644 --- a/server/nativeapi/translations_test.go +++ b/server/nativeapi/translations_test.go @@ -9,6 +9,7 @@ import ( "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/resources" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -16,6 +17,7 @@ import ( var _ = Describe("Translations", func() { Describe("I18n files", func() { It("contains only valid json language files", func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-nativeapi)") fsys := resources.FS() dir, _ := fsys.Open(consts.I18nFolder) files, _ := dir.(fs.ReadDirFile).ReadDir(-1) diff --git a/server/serve_index.go b/server/serve_index.go index bd5be44f5..734aabc70 100644 --- a/server/serve_index.go +++ b/server/serve_index.go @@ -45,7 +45,7 @@ func serveIndex(ds model.DataStore, fs fs.FS, shareInfo *model.Share) http.Handl "variousArtistsId": consts.VariousArtistsID, "baseURL": str.SanitizeText(strings.TrimSuffix(conf.Server.BasePath, "/")), "loginBackgroundURL": str.SanitizeText(conf.Server.UILoginBackgroundURL), - "welcomeMessage": str.SanitizeText(conf.Server.UIWelcomeMessage), + "welcomeMessage": str.SanitizeHTML(conf.Server.UIWelcomeMessage), "maxSidebarPlaylists": conf.Server.MaxSidebarPlaylists, "enableTranscodingConfig": conf.Server.EnableTranscodingConfig, "enableDownloads": conf.Server.EnableDownloads, diff --git a/server/serve_index_test.go b/server/serve_index_test.go index 7515e7276..31bca02cf 100644 --- a/server/serve_index_test.go +++ b/server/serve_index_test.go @@ -108,6 +108,18 @@ var _ = Describe("serveIndex", func() { Entry("extAuthLogoutURL", func() { conf.Server.ExtAuth.LogoutURL = "https://auth.example.com/logout" }, "extAuthLogoutURL", "https://auth.example.com/logout"), ) + It("sanitizes entity-encoded welcomeMessage as html", func() { + conf.Server.UIWelcomeMessage = `<img src=x onerror=alert(1)><b>Hello</b>` + r := httptest.NewRequest("GET", "/index.html", nil) + w := httptest.NewRecorder() + + serveIndex(ds, fs, nil)(w, r) + + config := extractAppConfig(w.Body.String()) + Expect(config).To(HaveKey("welcomeMessage")) + Expect(config["welcomeMessage"]).To(Equal(`Hello`)) + }) + DescribeTable("sets other UI configuration values", func(configKey string, expectedValueFunc func() any) { r := httptest.NewRequest("GET", "/index.html", nil) diff --git a/server/server_test.go b/server/server_test.go index 245fa013a..178c0015a 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -30,6 +30,7 @@ var _ = Describe("createUnixSocketFile", func() { When("unixSocketPerm is valid", func() { It("updates the permission of the unix socket file and returns nil", func() { + tests.SkipOnWindows("uses Unix file permission bits") _, err := createUnixSocketFile(socketPath, "0777") fileInfo, _ := os.Stat(socketPath) actualPermission := fileInfo.Mode().Perm() @@ -50,6 +51,7 @@ var _ = Describe("createUnixSocketFile", func() { When("file already exists", func() { It("recreates the file as a socket with the right permissions", func() { + tests.SkipOnWindows("uses Unix file permission bits") _, err := os.Create(socketPath) Expect(err).ToNot(HaveOccurred()) Expect(os.Chmod(socketPath, os.FileMode(0777))).To(Succeed()) diff --git a/server/subsonic/media_annotation_test.go b/server/subsonic/media_annotation_test.go index fc767b0ff..e110e2b93 100644 --- a/server/subsonic/media_annotation_test.go +++ b/server/subsonic/media_annotation_test.go @@ -32,7 +32,9 @@ var _ = Describe("MediaAnnotationController", func() { Describe("Scrobble", func() { It("submit all scrobbles with only the id", func() { - submissionTime := time.Now() + // Back-date the baseline so the assertion still passes on platforms + // with millisecond clock resolution (e.g. Windows). + submissionTime := time.Now().Add(-time.Second) r := newGetRequest("id=12", "id=34") _, err := router.Scrobble(r) diff --git a/tests/test_helpers.go b/tests/test_helpers.go index 0a2cad4ad..bdcd40d00 100644 --- a/tests/test_helpers.go +++ b/tests/test_helpers.go @@ -4,14 +4,25 @@ import ( "context" "os" "path/filepath" + "runtime" "github.com/navidrome/navidrome/db" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model/id" + "github.com/onsi/ginkgo/v2" "github.com/sirupsen/logrus" "github.com/sirupsen/logrus/hooks/test" ) +// SkipOnWindows marks the current spec (or surrounding BeforeEach) as skipped +// when running on Windows. The reason is included in the Ginkgo output so the +// backlog of Windows-skipped tests stays auditable. +func SkipOnWindows(reason string) { + if runtime.GOOS == "windows" { + ginkgo.Skip("not supported on Windows: " + reason) + } +} + type testingT interface { TempDir() string } @@ -20,10 +31,20 @@ func TempFileName(t testingT, prefix, suffix string) string { return filepath.Join(t.TempDir(), prefix+id.NewRandom()+suffix) } +// TempFile creates an empty file in t.TempDir() and returns the closed handle. +// The handle is returned for backward compatibility, but is already closed so +// callers don't need to. On Windows, leaving the handle open would hold a file +// lock and block Ginkgo's TempDir cleanup. func TempFile(t testingT, prefix, suffix string) (*os.File, string, error) { name := TempFileName(t, prefix, suffix) f, err := os.Create(name) - return f, name, err + if err != nil { + return nil, name, err + } + if cerr := f.Close(); cerr != nil { + return f, name, cerr + } + return f, name, nil } // ClearDB deletes all tables and data from the database diff --git a/utils/files_test.go b/utils/files_test.go index 72fc4f96f..c6e578f05 100644 --- a/utils/files_test.go +++ b/utils/files_test.go @@ -192,6 +192,10 @@ var _ = Describe("FileExists", func() { filePath := tempFile.Name() Expect(utils.FileExists(filePath)).To(BeTrue()) + // Close the file before removing it. On Windows, an open handle + // holds a file lock and os.Remove fails; closing first makes the + // test cross-platform. + Expect(tempFile.Close()).To(Succeed()) err := os.Remove(filePath) Expect(err).NotTo(HaveOccurred()) tempFile = nil // Prevent cleanup attempt diff --git a/utils/str/sanitize_strings.go b/utils/str/sanitize_strings.go index 73608112e..c121aefe7 100644 --- a/utils/str/sanitize_strings.go +++ b/utils/str/sanitize_strings.go @@ -38,11 +38,20 @@ func SanitizeStrings(text ...string) string { var policy = bluemonday.UGCPolicy() +// SanitizeText unescapes the input string before sanitizing it as text. +// This should be used for fields rendered as plain text in the UI (e.g. lyrics, song titles, artist names) func SanitizeText(text string) string { s := policy.Sanitize(text) return html.UnescapeString(s) } +// SanitizeHTML unescapes the input string before sanitizing it as HTML. +// This should be used for fields rendered as HTML by clients (e.g. biographies, welcome messages) +// to prevent XSS bypasses via entity-encoded tags. +func SanitizeHTML(text string) string { + return policy.Sanitize(html.UnescapeString(text)) +} + func SanitizeFieldForSorting(originalValue string) string { v := strings.TrimSpace(sanitize.Accents(originalValue)) return Clear(strings.ToLower(v)) diff --git a/utils/str/sanitize_strings_test.go b/utils/str/sanitize_strings_test.go index ac28fe435..6527f326b 100644 --- a/utils/str/sanitize_strings_test.go +++ b/utils/str/sanitize_strings_test.go @@ -64,6 +64,35 @@ var _ = Describe("Sanitize Strings", func() { }) }) + Describe("SanitizeText", func() { + It("preserves decoded plaintext", func() { + Expect(str.SanitizeText("Tom & Jerry")).To(Equal("Tom & Jerry")) + Expect(str.SanitizeText("Tom & Jerry")).To(Equal("Tom & Jerry")) + }) + + It("keeps entity-encoded html readable", func() { + Expect(str.SanitizeText(`<b>ok</b>`)).To(Equal("ok")) + }) + }) + + Describe("SanitizeHTML", func() { + It("removes dangerous content from raw html", func() { + sanitized := str.SanitizeHTML(`ok`) + + Expect(sanitized).To(ContainSubstring("ok")) + Expect(sanitized).ToNot(ContainSubstring("onerror")) + Expect(sanitized).ToNot(ContainSubstring("ok")) + Expect(sanitized).ToNot(ContainSubstring("onerror")) + Expect(sanitized).ToNot(ContainSubstring("