mirrors/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2025-04-04 04:57:37 +03:00
Code Issues Projects Releases Packages Wiki Activity

Refactor

Browse source
This commit is contained in:
Deluan 2023-05-22 11:59:33 -04:00 • committed by Deluan
parent 09e52eba87
commit ea231fe265
8 changed files with 142 additions and 90 deletions
Download patch file Download diff file Expand all files Collapse all files

15
Makefile
View file

@ -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

1
server/api/api.go
View file

@ -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"

49
server/api/helpers.go
View file

@ -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)
}

35
server/api/helpers_test.go
View file

@ -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"},

30
server/middlewares.go
View file

@ -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.

15
server/middlewares_test.go
View file

@ -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)

39
utils/gg/gg.go
View file

@ -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
}

48
utils/gg/gg_test.go
View file

@ -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(""))
})
})
})