diff --git a/Makefile b/Makefile index 5f3189dc4..de9efaf2a 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,11 @@ test: ##@Development Run Go tests go test -race -shuffle=on ./... .PHONY: test -testall: test ##@Development Run Go and JS tests +testapi: api/openapi.yaml ##@Development Validate OpenAPI spec + @npx swagger-cli validate api/openapi.yaml +.PHONY: testapi + +testall: testapi test ##@Development Run Go and JS tests, and validate OpenAPI spec @(cd ./ui && npm test -- --watchAll=false) .PHONY: testall @@ -45,14 +49,19 @@ lintall: lint ##@Development Lint Go and JS code @(cd ./ui && npm run lint) .PHONY: lintall -wire: check_go_env ##@Development Update Dependency Injection +gen: check_go_env api ##@Development Update Generated Code (wire, mocks, openapi, etc) go run github.com/google/wire/cmd/wire@latest ./... .PHONY: wire -api: check_go_env ##@Development Generate New Native API Server Boilerplate +api: check_go_env api/openapi.yaml go generate ./server/api/... .PHONY: api +spec_parts=$(shell find api -name '*.yml') +api/openapi.yaml: $(spec_parts) + @echo "Bundling OpenAPI spec..." + npx swagger-cli bundle api/spec.yml --outfile api/openapi.yaml --type yaml + snapshots: ##@Development Update (GoLang) Snapshot tests UPDATE_SNAPSHOTS=true go run github.com/onsi/ginkgo/v2/ginkgo@latest ./server/subsonic/... .PHONY: snapshots diff --git a/server/api/api.go b/server/api/api.go index 304e6c5c6..bc3e2e619 100644 --- a/server/api/api.go +++ b/server/api/api.go @@ -1,4 +1,3 @@ -//go:generate npx swagger-cli bundle ../../api/spec.yml --outfile ../../api/openapi.yaml --type yaml //go:generate go run github.com/deepmap/oapi-codegen/cmd/oapi-codegen -config ./openapi_api.cfg.yaml "../../api/openapi.yaml" //go:generate go run github.com/deepmap/oapi-codegen/cmd/oapi-codegen -config ./openapi_types.cfg.yaml "../../api/openapi.yaml" diff --git a/server/api/helpers.go b/server/api/helpers.go index 632e8471b..5bd2e51e7 100644 --- a/server/api/helpers.go +++ b/server/api/helpers.go @@ -15,6 +15,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server" + . "github.com/navidrome/navidrome/utils/gg" ) type contextKey string @@ -38,19 +39,19 @@ func toAPITrack(mf model.MediaFile) Track { Albumartist: mf.AlbumArtist, Artist: mf.Artist, Bitrate: mf.BitRate, - Bpm: p(mf.Bpm), + Bpm: P(mf.Bpm), Channels: mf.Channels, - Comments: p(mf.Comment), - Disc: p(mf.DiscNumber), + Comments: P(mf.Comment), + Disc: P(mf.DiscNumber), Duration: mf.Duration, - Genre: p(mf.Genre), + Genre: P(mf.Genre), Mimetype: mf.ContentType(), - RecordingMbid: p(mf.MbzTrackID), + RecordingMbid: P(mf.MbzTrackID), Size: int(mf.Size), Title: mf.Title, Track: mf.TrackNumber, - TrackMbid: p(mf.MbzReleaseTrackID), - Year: p(mf.Year), + TrackMbid: P(mf.MbzReleaseTrackID), + Year: P(mf.Year), }, Relationships: &TrackRelationships{ Albums: &[]AlbumTrackRelationship{toAlbumRelationship(mf)}, @@ -97,22 +98,6 @@ func toAPITracks(mfs model.MediaFiles) []Track { return tracks } -func p[T comparable](t T) *T { - var zero T - if t == zero { - return nil - } - return &t -} - -func v[T comparable](p *T) T { - var zero T - if p == nil { - return zero - } - return *p -} - // toQueryOptions convert a params struct to a model.QueryOptions struct, to be used by the // GetAll and CountAll functions. It assumes all GetXxxxParams functions have the exact same structure. func toQueryOptions(ctx context.Context, params GetTracksParams) model.QueryOptions { @@ -133,8 +118,8 @@ func toQueryOptions(ctx context.Context, params GetTracksParams) model.QueryOpti parseFilter(params.FilterGreaterOrEqual, func(f, v string) squirrel.Sqlizer { return squirrel.GtOrEq{f: v} }) parseFilter(params.FilterLessThan, func(f, v string) squirrel.Sqlizer { return squirrel.Lt{f: v} }) parseFilter(params.FilterLessOrEqual, func(f, v string) squirrel.Sqlizer { return squirrel.LtOrEq{f: v} }) - offset := v(params.PageOffset) - limit := v(params.PageLimit) + offset := V(params.PageOffset) + limit := V(params.PageLimit) sort, err := toSortParams(params.Sort) if err != nil { log.Warn(ctx, "Ignoring invalid sort parameter", err) @@ -177,15 +162,15 @@ func toSortParams(sort *string) (string, error) { return strings.Join(resultCols, ","), nil } -func apiErrorHandler(w http.ResponseWriter, r *http.Request, err error) { +func apiErrorHandler(w http.ResponseWriter, _ *http.Request, err error) { var res ErrorObject switch { case errors.Is(err, model.ErrNotAuthorized): - res = ErrorObject{Status: p(strconv.Itoa(http.StatusForbidden)), Title: p(http.StatusText(http.StatusForbidden))} + res = ErrorObject{Status: P(strconv.Itoa(http.StatusForbidden)), Title: P(http.StatusText(http.StatusForbidden))} case errors.Is(err, model.ErrNotFound): - res = ErrorObject{Status: p(strconv.Itoa(http.StatusNotFound)), Title: p(http.StatusText(http.StatusNotFound))} + res = ErrorObject{Status: P(strconv.Itoa(http.StatusNotFound)), Title: P(http.StatusText(http.StatusNotFound))} default: - res = ErrorObject{Status: p(strconv.Itoa(http.StatusInternalServerError)), Title: p(http.StatusText(http.StatusInternalServerError))} + res = ErrorObject{Status: P(strconv.Itoa(http.StatusInternalServerError)), Title: P(http.StatusText(http.StatusInternalServerError))} } w.Header().Set("Content-Type", "application/vnd.api+json") w.WriteHeader(403) @@ -196,9 +181,9 @@ func apiErrorHandler(w http.ResponseWriter, r *http.Request, err error) { func validationErrorHandler(w http.ResponseWriter, message string, statusCode int) { _ = GetTracks400JSONResponse{BadRequestJSONResponse{Errors: []ErrorObject{ { - Status: p(strconv.Itoa(statusCode)), - Title: p(http.StatusText(statusCode)), - Detail: p(message), + Status: P(strconv.Itoa(statusCode)), + Title: P(http.StatusText(statusCode)), + Detail: P(message), }, }}}.VisitGetTracksResponse(w) } diff --git a/server/api/helpers_test.go b/server/api/helpers_test.go index 57afbe3bc..fddb7b70f 100644 --- a/server/api/helpers_test.go +++ b/server/api/helpers_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "net/url" + . "github.com/navidrome/navidrome/utils/gg" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -34,44 +35,44 @@ var _ = Describe("BuildPaginationLinksAndMeta", func() { It("returns correct pagination links and meta", func() { links, meta := buildPaginationLinksAndMeta(totalItems, params, resourceName) - testLinkEquality(links.First, p("api/resource?page[offset]=0&page[limit]=10")) - testLinkEquality(links.Last, p("api/resource?page[offset]=140&page[limit]=10")) - testLinkEquality(links.Next, p("api/resource?page[offset]=10&page[limit]=10")) + testLinkEquality(links.First, P("api/resource?page[offset]=0&page[limit]=10")) + testLinkEquality(links.Last, P("api/resource?page[offset]=140&page[limit]=10")) + testLinkEquality(links.Next, P("api/resource?page[offset]=10&page[limit]=10")) Expect(links.Prev).To(BeNil()) - Expect(meta.CurrentPage).To(Equal(p(int32(1)))) - Expect(meta.TotalItems).To(Equal(p(int32(150)))) - Expect(meta.TotalPages).To(Equal(p(int32(15)))) + Expect(meta.CurrentPage).To(Equal(P(int32(1)))) + Expect(meta.TotalItems).To(Equal(P(int32(150)))) + Expect(meta.TotalPages).To(Equal(P(int32(15)))) }) }) Context("with custom page limit and offset", func() { BeforeEach(func() { params = GetTracksParams{ - PageLimit: p((PageLimit)(20)), - PageOffset: p((PageOffset)(40)), + PageLimit: P((PageLimit)(20)), + PageOffset: P((PageOffset)(40)), } }) It("returns correct pagination links and meta", func() { links, meta := buildPaginationLinksAndMeta(totalItems, params, resourceName) - testLinkEquality(links.First, p("api/resource?page[offset]=0&page[limit]=20")) - testLinkEquality(links.Last, p("api/resource?page[offset]=140&page[limit]=20")) - testLinkEquality(links.Next, p("api/resource?page[offset]=60&page[limit]=20")) - testLinkEquality(links.Prev, p("api/resource?page[offset]=20&page[limit]=20")) + testLinkEquality(links.First, P("api/resource?page[offset]=0&page[limit]=20")) + testLinkEquality(links.Last, P("api/resource?page[offset]=140&page[limit]=20")) + testLinkEquality(links.Next, P("api/resource?page[offset]=60&page[limit]=20")) + testLinkEquality(links.Prev, P("api/resource?page[offset]=20&page[limit]=20")) - Expect(meta.CurrentPage).To(Equal(p(int32(3)))) - Expect(meta.TotalItems).To(Equal(p(int32(150)))) - Expect(meta.TotalPages).To(Equal(p(int32(8)))) + Expect(meta.CurrentPage).To(Equal(P(int32(3)))) + Expect(meta.TotalItems).To(Equal(P(int32(150)))) + Expect(meta.TotalPages).To(Equal(P(int32(8)))) }) }) Context("with various filter params", func() { BeforeEach(func() { params = GetTracksParams{ - PageLimit: p((PageLimit)(20)), - PageOffset: p((PageOffset)(40)), + PageLimit: P((PageLimit)(20)), + PageOffset: P((PageOffset)(40)), FilterEquals: &[]string{"property1:value1", "property2:value2"}, FilterContains: &[]string{"property3:value3"}, FilterLessThan: &[]string{"property4:value4"}, diff --git a/server/middlewares.go b/server/middlewares.go index cdbefacb8..8591d3de0 100644 --- a/server/middlewares.go +++ b/server/middlewares.go @@ -190,41 +190,31 @@ var ( // handling the given request, as determined by the presence of X-Forwarded-* headers // or the scheme and host of the request URL. func serverAddress(r *http.Request) (scheme, host string) { - // Save the original request host for later comparison. - origHost := r.Host - // Determine the protocol of the request based on the presence of a TLS connection. protocol := "http" if r.TLS != nil { protocol = "https" } - // Get the X-Forwarded-Host header and extract the first host name if there are - // multiple hosts listed. If there is no X-Forwarded-Host header, use the original - // request host as the default. - xfh := r.Header.Get(xForwardedHost) - if xfh != "" { - i := strings.Index(xfh, ",") - if i == -1 { - i = len(xfh) - } - xfh = xfh[:i] - } - host = FirstOr(r.Host, xfh) - // Determine the protocol and scheme of the request based on the presence of // X-Forwarded-* headers or the scheme of the request URL. - scheme = FirstOr( - protocol, + scheme = Coalesce( r.Header.Get(xForwardedProto), r.Header.Get(xForwardedScheme), r.URL.Scheme, + protocol, ) + // Get the X-Forwarded-Host header and extract the first host name if there are + // multiple hosts listed. If there is no X-Forwarded-Host header, use the original + // request host as the default. + parts := strings.Split(r.Header.Get(xForwardedHost), ",") + host = Coalesce(parts[0], r.Host) + // If the request host has changed due to the X-Forwarded-Host header, log a trace // message with the original and new host values, as well as the scheme and URL. - if host != origHost { - log.Trace(r.Context(), "Request host has changed", "origHost", origHost, "host", host, "scheme", scheme, "url", r.URL) + if host != r.Host { + log.Trace(r.Context(), "Request host has changed", "origHost", r.Host, "host", host, "scheme", scheme, "url", r.URL) } // Return the scheme and host of the server handling the request. diff --git a/server/middlewares_test.go b/server/middlewares_test.go index 7823b1ef8..28ebe0971 100644 --- a/server/middlewares_test.go +++ b/server/middlewares_test.go @@ -86,7 +86,7 @@ var _ = Describe("middlewares", func() { }) }) - Context("with X-Forwarded-Host header", func() { + Context("with X-Forwarded-Host header with one host", func() { BeforeEach(func() { req, _ = http.NewRequest("GET", "http://example.com", nil) req.Header.Set("X-Forwarded-Host", "forwarded.example.com") @@ -99,6 +99,19 @@ var _ = Describe("middlewares", func() { }) }) + Context("with X-Forwarded-Host header with multiple hosts", func() { + BeforeEach(func() { + req, _ = http.NewRequest("GET", "http://example.com", nil) + req.Header.Set("X-Forwarded-Host", "forwarded.example.com,forwarded2.example.com") + }) + + It("should modify the request with the X-Forwarded-Host header value", func() { + middleware.ServeHTTP(recorder, req) + Expect(req.Host).To(Equal("forwarded.example.com")) + Expect(req.URL.Scheme).To(Equal("http")) + }) + }) + Context("with X-Forwarded-Proto header", func() { BeforeEach(func() { req, _ = http.NewRequest("GET", "http://example.com", nil) diff --git a/utils/gg/gg.go b/utils/gg/gg.go index 6da9082a1..95b2aa1e3 100644 --- a/utils/gg/gg.go +++ b/utils/gg/gg.go @@ -13,20 +13,37 @@ func If[T comparable](v T, orElse T) T { return orElse } -// FirstOr is a generic helper function that returns the first non-zero value from -// a list of comparable values, or a default value if all the values are zero. -func FirstOr[T comparable](or T, values ...T) T { - // Initialize a zero value of the same type as the input values. +// P returns a pointer to a value, or nil if the value is the zero value of the type. +func P[T comparable](t T) *T { var zero T + if t == zero { + return nil + } + return &t +} - // Loop through each input value and check if it is non-zero. If a non-zero value - // is found, return it immediately. - for _, v := range values { +// V returns the value of a pointer, or the zero value of the type if the pointer is nil. +func V[T comparable](p *T) T { + var zero T + if p == nil { + return zero + } + return *p +} + +// Coalesce returns the first non-zero value from listed arguments. +// Returns the zero value of the type parameter if no arguments are given or all are the zero value. +// Useful when you want to initialize a variable to the first non-zero value from a list of fallback values. +// +// For example: +// +// hostVal := Coalesce(hostName, os.Getenv("HOST"), "localhost") +func Coalesce[T comparable](values ...T) (v T) { + var zero T + for _, v = range values { if v != zero { - return v + return } } - - // If all the input values are zero, return the default value. - return or + return } diff --git a/utils/gg/gg_test.go b/utils/gg/gg_test.go index e492fae9d..c82079b69 100644 --- a/utils/gg/gg_test.go +++ b/utils/gg/gg_test.go @@ -43,16 +43,54 @@ var _ = Describe("GG", func() { ) }) - Describe("FirstOr", func() { + Describe("Coalesce", func() { Context("when given a list of strings", func() { It("returns the first non-empty value", func() { - Expect(gg.FirstOr("default", "foo", "bar", "baz")).To(Equal("foo")) - Expect(gg.FirstOr("default", "", "", "qux")).To(Equal("qux")) + Expect(gg.Coalesce("foo", "bar", "baz", "default")).To(Equal("foo")) + Expect(gg.Coalesce("", "", "qux", "default")).To(Equal("qux")) }) It("returns the default value if all values are empty", func() { - Expect(gg.FirstOr("default", "", "", "")).To(Equal("default")) - Expect(gg.FirstOr("", "", "", "")).To(Equal("")) + Expect(gg.Coalesce("", "", "", "default")).To(Equal("default")) + Expect(gg.Coalesce("", "", "", "")).To(Equal("")) + }) + }) + }) + + Describe("P", func() { + Context("when given a non-zero value", func() { + It("should return a non-nil pointer to that value", func() { + value := 42 + result := gg.P(value) + Expect(result).ToNot(BeNil()) + Expect(*result).To(Equal(value)) + }) + }) + + Context("when given the zero value of a type", func() { + It("should return nil", func() { + var value string + result := gg.P(value) + Expect(result).To(BeNil()) + }) + }) + }) + + Describe("V", func() { + Context("when given a non-nil pointer", func() { + It("should return the value of that pointer", func() { + value := 42 + pointer := &value + result := gg.V(pointer) + Expect(result).To(Equal(value)) + }) + }) + + Context("when given a nil pointer", func() { + It("should return the zero value of the type", func() { + var pointer *string + result := gg.V(pointer) + Expect(result).To(Equal("")) }) }) })