diff --git a/.gitignore b/.gitignore index 27b23240..c04f5b15 100644 --- a/.gitignore +++ b/.gitignore @@ -24,4 +24,5 @@ docker-compose.yml !contrib/docker-compose.yml binaries navidrome-master -*.exe \ No newline at end of file +*.exe +bin/ \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..272426e7 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,110 @@ +# Testing Instructions + +- **No implementation task is considered complete until it includes thorough, passing tests that cover the new or + changed functionality. All new code must be accompanied by Ginkgo/Gomega tests, and PRs/commits without tests should + be considered incomplete.** +- All Go tests in this project **MUST** be written using the **Ginkgo v2** and **Gomega** frameworks. +- To run all tests, use `make test`. +- To run tests for a specific package, use `make test PKG=./pkgname/...` +- Do not run tests in parallel +- Don't use `--fail-on-pending` + +## Mocking Convention + +- Always try to use the mocks provided in the `tests` package before creating a new mock implementation. +- Only create a new mock if the required functionality is not covered by the existing mocks in `tests`. +- Never mock a real implementation when testing. Remember: there is no value in testing an interface, only the real implementation. + +## Example + +Every package that you write tests for, should have a `*_suite_test.go` file, to hook up the Ginkgo test suite. Example: +``` +package core + +import ( + "testing" + + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestCore(t *testing.T) { + tests.Init(t, false) + log.SetLevel(log.LevelFatal) + RegisterFailHandler(Fail) + RunSpecs(t, "Core Suite") +} +``` +Never put a `func Test*` in regular *_test.go files, only in `*_suite_test.go` files. + +Refer to existing test suites for examples of proper setup and usage, such as the one defined in @core_suite_test.go + +## Exceptions + +There should be no exceptions to this rule. If you encounter tests written with the standard `testing` package or other frameworks, they should be refactored to use Ginkgo/Gomega. If you need a new mock, first confirm that it does not already exist in the `tests` package. + +### Configuration + +You can set config values in the BeforeEach/BeforeAll blocks. If you do so, remember to add `DeferCleanup(configtest.SetupConfig())` to reset the values. Example: + +```go +BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.EnableDownloads = true +}) +``` + +# Logging System Usage Guide + +This project uses a custom logging system built on top of logrus, `log/log.go`. Follow these conventions for all logging: + +## Logging API +- Use the provided functions for logging at different levels: + - `Error(...)`, `Warn(...)`, `Info(...)`, `Debug(...)`, `Trace(...)`, `Fatal(...)` +- These functions accept flexible arguments: + - The first argument can be a context (`context.Context`), an HTTP request, or `nil`. + - The next argument is the log message (string or error). + - Additional arguments are key-value pairs (e.g., `"key", value`). + - If the last argument is an error, it is logged under the `error` key. + +**Examples:** +```go +log.Error("A message") +log.Error(ctx, "A message with context") +log.Error("Failed to save", "id", 123, err) +log.Info(req, "Request received", "user", userID) +``` + +## Logging errors +- You don't need to add "err" key when logging an error, it is automatically added. +- Error must always be the last parameter in the log call. + Examples: +```go +log.Error("Failed to save", "id", 123, err) // GOOD +log.Error("Failed to save", "id", 123, "err", err) // BAD +log.Error("Failed to save", err, "id", 123) // BAD +``` + +## Context and Request Logging +- If a context or HTTP request is passed as the first argument, any logger fields in the context are included in the log entry. +- Use `log.NewContext(ctx, "key", value, ...)` to add fields to a context for logging. + +## Log Levels +- Set the global log level with `log.SetLevel(log.LevelInfo)` or `log.SetLevelString("info")`. +- Per-path log levels can be set with `log.SetLogLevels(map[string]string{"path": "level"})`. +- Use `log.IsGreaterOrEqualTo(level)` to check if a log level is enabled for the current code path. + +## Source Line Logging +- Enable source file/line logging with `log.SetLogSourceLine(true)`. + +## Best Practices +- Always use the logging API, never log directly with logrus or fmt. +- Prefer structured logging (key-value pairs) for important data. +- Use context/request logging for traceability in web handlers. +- For tests, use Ginkgo/Gomega and set up a test logger as in `log/log_test.go`. + +## See Also +- `log/log.go` for implementation details +- `log/log_test.go` for usage examples and test patterns diff --git a/model/tag.go b/model/tag.go index a9864e0b..a1f4e28d 100644 --- a/model/tag.go +++ b/model/tag.go @@ -191,6 +191,7 @@ const ( TagReleaseCountry TagName = "releasecountry" TagMedia TagName = "media" TagCatalogNumber TagName = "catalognumber" + TagISRC TagName = "isrc" TagBPM TagName = "bpm" TagExplicitStatus TagName = "explicitstatus" diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index 4faec158..39f32465 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -224,6 +224,7 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op child.BPM = int32(mf.BPM) child.MediaType = responses.MediaTypeSong child.MusicBrainzId = mf.MbzRecordingID + child.Isrc = mf.Tags.Values(model.TagISRC) child.ReplayGain = responses.ReplayGain{ TrackGain: mf.RGTrackGain, AlbumGain: mf.RGAlbumGain, diff --git a/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .JSON b/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .JSON index 0db35c37..0e6425f6 100644 --- a/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .JSON +++ b/server/subsonic/responses/.snapshots/Responses AlbumList with OS data should match .JSON @@ -15,6 +15,7 @@ "sortName": "sort name", "mediaType": "album", "musicBrainzId": "00000000-0000-0000-0000-000000000000", + "isrc": [], "genres": [ { "name": "Genre 1" diff --git a/server/subsonic/responses/.snapshots/Responses AlbumWithSongsID3 with data should match .JSON b/server/subsonic/responses/.snapshots/Responses AlbumWithSongsID3 with data should match .JSON index c3ae3ee2..78b5c6e7 100644 --- a/server/subsonic/responses/.snapshots/Responses AlbumWithSongsID3 with data should match .JSON +++ b/server/subsonic/responses/.snapshots/Responses AlbumWithSongsID3 with data should match .JSON @@ -99,6 +99,9 @@ "sortName": "sorted song", "mediaType": "song", "musicBrainzId": "4321", + "isrc": [ + "ISRC-1" + ], "genres": [ { "name": "rock" diff --git a/server/subsonic/responses/.snapshots/Responses AlbumWithSongsID3 with data should match .XML b/server/subsonic/responses/.snapshots/Responses AlbumWithSongsID3 with data should match .XML index a02c0fee..f3281d9e 100644 --- a/server/subsonic/responses/.snapshots/Responses AlbumWithSongsID3 with data should match .XML +++ b/server/subsonic/responses/.snapshots/Responses AlbumWithSongsID3 with data should match .XML @@ -16,6 +16,7 @@ + ISRC-1 diff --git a/server/subsonic/responses/.snapshots/Responses Child with data should match .JSON b/server/subsonic/responses/.snapshots/Responses Child with data should match .JSON index 13aa1f18..d64ae9e7 100644 --- a/server/subsonic/responses/.snapshots/Responses Child with data should match .JSON +++ b/server/subsonic/responses/.snapshots/Responses Child with data should match .JSON @@ -30,6 +30,10 @@ "sortName": "sorted title", "mediaType": "song", "musicBrainzId": "4321", + "isrc": [ + "ISRC-1", + "ISRC-2" + ], "genres": [ { "name": "rock" diff --git a/server/subsonic/responses/.snapshots/Responses Child with data should match .XML b/server/subsonic/responses/.snapshots/Responses Child with data should match .XML index 477892ac..639fd3f6 100644 --- a/server/subsonic/responses/.snapshots/Responses Child with data should match .XML +++ b/server/subsonic/responses/.snapshots/Responses Child with data should match .XML @@ -1,6 +1,8 @@ + ISRC-1 + ISRC-2 diff --git a/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .JSON b/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .JSON index 5dc0e8eb..1af2ec4a 100644 --- a/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .JSON +++ b/server/subsonic/responses/.snapshots/Responses Child without data should match OpenSubsonic .JSON @@ -15,6 +15,7 @@ "sortName": "", "mediaType": "", "musicBrainzId": "", + "isrc": [], "genres": [], "replayGain": {}, "channelCount": 0, diff --git a/server/subsonic/responses/responses.go b/server/subsonic/responses/responses.go index 0d22ef50..e886d656 100644 --- a/server/subsonic/responses/responses.go +++ b/server/subsonic/responses/responses.go @@ -176,6 +176,7 @@ type OpenSubsonicChild struct { SortName string `xml:"sortName,attr,omitempty" json:"sortName"` MediaType MediaType `xml:"mediaType,attr,omitempty" json:"mediaType"` MusicBrainzId string `xml:"musicBrainzId,attr,omitempty" json:"musicBrainzId"` + Isrc Array[string] `xml:"isrc,omitempty" json:"isrc"` Genres Array[ItemGenre] `xml:"genres,omitempty" json:"genres"` ReplayGain ReplayGain `xml:"replayGain,omitempty" json:"replayGain"` ChannelCount int32 `xml:"channelCount,attr,omitempty" json:"channelCount"` diff --git a/server/subsonic/responses/responses_test.go b/server/subsonic/responses/responses_test.go index e484ab2c..9fcd6078 100644 --- a/server/subsonic/responses/responses_test.go +++ b/server/subsonic/responses/responses_test.go @@ -224,7 +224,8 @@ var _ = Describe("Responses", func() { child[0].OpenSubsonicChild = &OpenSubsonicChild{ Genres: []ItemGenre{{Name: "rock"}, {Name: "progressive"}}, Comment: "a comment", MediaType: MediaTypeSong, MusicBrainzId: "4321", SortName: "sorted title", - BPM: 127, ChannelCount: 2, SamplingRate: 44100, BitDepth: 16, + Isrc: []string{"ISRC-1", "ISRC-2"}, + BPM: 127, ChannelCount: 2, SamplingRate: 44100, BitDepth: 16, Moods: []string{"happy", "sad"}, ReplayGain: ReplayGain{TrackGain: 1, AlbumGain: 2, TrackPeak: 3, AlbumPeak: 4, BaseGain: 5, FallbackGain: 6}, DisplayArtist: "artist 1 & artist 2", @@ -312,6 +313,7 @@ var _ = Describe("Responses", func() { songs[0].OpenSubsonicChild = &OpenSubsonicChild{ Genres: []ItemGenre{{Name: "rock"}, {Name: "progressive"}}, Comment: "a comment", MediaType: MediaTypeSong, MusicBrainzId: "4321", SortName: "sorted song", + Isrc: []string{"ISRC-1"}, Moods: []string{"happy", "sad"}, ReplayGain: ReplayGain{TrackGain: 1, AlbumGain: 2, TrackPeak: 3, AlbumPeak: 4, BaseGain: 5, FallbackGain: 6}, BPM: 127, ChannelCount: 2, SamplingRate: 44100, BitDepth: 16,