From 26457aae6c5ff6f86d1a273f8b40f91b8e46895d Mon Sep 17 00:00:00 2001 From: sentriz Date: Wed, 29 Apr 2020 20:10:18 +0100 Subject: [PATCH] add const --- server/ctrlsubsonic/handlers_by_tags.go | 2 +- server/ctrlsubsonic/handlers_raw.go | 2 +- server/ctrlsubsonic/params/params.go | 143 +++++------------- server/ctrlsubsonic/params/params_test.go | 61 -------- .../ctrlsubsonic/spec/construct_by_folder.go | 19 ++- server/ctrlsubsonic/spec/spec.go | 24 +-- server/ids/ids.go | 59 ++++++++ server/ids/ids_test.go | 35 +++++ 8 files changed, 157 insertions(+), 188 deletions(-) delete mode 100644 server/ctrlsubsonic/params/params_test.go create mode 100644 server/ids/ids.go create mode 100644 server/ids/ids_test.go diff --git a/server/ctrlsubsonic/handlers_by_tags.go b/server/ctrlsubsonic/handlers_by_tags.go index d7fe0f5..f99302b 100644 --- a/server/ctrlsubsonic/handlers_by_tags.go +++ b/server/ctrlsubsonic/handlers_by_tags.go @@ -210,7 +210,7 @@ func (c *Controller) ServeSearchThree(r *http.Request) *spec.Response { func (c *Controller) ServeGetArtistInfoTwo(r *http.Request) *spec.Response { params := r.Context().Value(CtxParams).(params.Params) - id, err := params.GetIDDefault() + id, err := params.GetInt("id") if err != nil { return spec.NewError(10, "please provide an `id` parameter") } diff --git a/server/ctrlsubsonic/handlers_raw.go b/server/ctrlsubsonic/handlers_raw.go index 6db62a3..9cc1895 100644 --- a/server/ctrlsubsonic/handlers_raw.go +++ b/server/ctrlsubsonic/handlers_raw.go @@ -82,7 +82,7 @@ func (c *Controller) ServeGetCoverArt(w http.ResponseWriter, r *http.Request) *s func (c *Controller) ServeStream(w http.ResponseWriter, r *http.Request) *spec.Response { params := r.Context().Value(CtxParams).(params.Params) - id, err := params.GetInt("id") + id, err := params.GetID("id") if err != nil { return spec.NewError(10, "please provide an `id` parameter") } diff --git a/server/ctrlsubsonic/params/params.go b/server/ctrlsubsonic/params/params.go index 1acef90..ffe9816 100644 --- a/server/ctrlsubsonic/params/params.go +++ b/server/ctrlsubsonic/params/params.go @@ -26,45 +26,23 @@ package params import ( "errors" - "fmt" "net/http" "net/url" "strconv" - "strings" + + "go.senan.xyz/gonic/server/ids" ) -var ( - ErrKeyNotFound = errors.New("key(s) not found") - ErrIDInvalid = errors.New("invalid id") - ErrIDNotAnInt = errors.New("not an int") - ErrIDNotABool = errors.New("not an bool") -) - -const IDSeparator = "-" - -type IDType string - -const ( - // type values copied from subsonic - IDTypeArtist IDType = "ar" - IDTypeAlbum IDType = "al" - IDTypeTrack IDType = "tr" -) - -type ID struct { - Type IDType - Value int -} - -func IDArtist(id int) string { return fmt.Sprintf("%d-%s", id, IDTypeArtist) } -func IDAlbum(id int) string { return fmt.Sprintf("%d-%s", id, IDTypeAlbum) } -func IDTrack(id int) string { return fmt.Sprintf("%d-%s", id, IDTypeTrack) } - -// ** begin type parsing, support {[],}{string,int,ID} => 6 types +// some thin wrappers +// may be needed when cleaning up parse() below +func parseStr(in string) (string, error) { return in, nil } +func parseInt(in string) (int, error) { return strconv.Atoi(in) } +func parseID(in string) (ids.IDV, error) { return ids.Parse(in) } +func parseBool(in string) (bool, error) { return strconv.ParseBool(in) } func parse(values []string, i interface{}) error { if len(values) == 0 { - return ErrKeyNotFound + return errors.New("no values provided") } var err error switch v := i.(type) { @@ -72,37 +50,37 @@ func parse(values []string, i interface{}) error { *v, err = parseStr(values[0]) case *int: *v, err = parseInt(values[0]) - case *ID: + case *ids.IDV: *v, err = parseID(values[0]) case *bool: *v, err = parseBool(values[0]) case *[]string: - for _, val := range values { - parsed, err := parseStr(val) + for _, value := range values { + parsed, err := parseStr(value) if err != nil { return err } *v = append(*v, parsed) } case *[]int: - for _, val := range values { - parsed, err := parseInt(val) + for _, value := range values { + parsed, err := parseInt(value) if err != nil { return err } *v = append(*v, parsed) } - case *[]ID: - for _, val := range values { - parsed, err := parseID(val) + case *[]ids.IDV: + for _, value := range values { + parsed, err := parseID(value) if err != nil { return err } *v = append(*v, parsed) } case *[]bool: - for _, val := range values { - parsed, err := parseBool(val) + for _, value := range values { + parsed, err := parseBool(value) if err != nil { return err } @@ -112,48 +90,6 @@ func parse(values []string, i interface{}) error { return err } -// ** begin parse funcs - -func parseStr(in string) (string, error) { - return in, nil -} - -func parseInt(in string) (int, error) { - if v, err := strconv.Atoi(in); err == nil { - return v, nil - } - return 0, ErrIDNotAnInt -} - -func parseID(in string) (ID, error) { - parts := strings.Split(in, IDSeparator) - if len(parts) != 2 { - return ID{}, fmt.Errorf("bad separator: %w", ErrIDInvalid) - } - partType := parts[0] - partValue := parts[1] - val, err := parseInt(partValue) - if err != nil { - return ID{}, fmt.Errorf("%s: %w", partValue, err) - } - switch partType { - case string(IDTypeArtist): - return ID{Type: IDTypeArtist, Value: val}, nil - case string(IDTypeAlbum): - return ID{Type: IDTypeAlbum, Value: val}, nil - case string(IDTypeTrack): - return ID{Type: IDTypeTrack, Value: val}, nil - } - return ID{}, ErrIDInvalid -} - -func parseBool(in string) (bool, error) { - if v, err := strconv.ParseBool(in); err == nil { - return v, nil - } - return false, ErrIDNotABool -} - type Params url.Values func New(r *http.Request) Params { @@ -293,61 +229,56 @@ func (p Params) GetFirstOrIntList(or []int, keys ...string) []int { return or } -// ** begin ID {get, get first, get or, get first or} +// ** begin ids.IDV {get, get first, get or, get first or} -func (p Params) GetID(key string) (ID, error) { - var ret ID +func (p Params) GetID(key string) (ids.IDV, error) { + var ret ids.IDV return ret, parse(p.get(key), &ret) } -func (p Params) GetIDDefault() (ID, error) { - var ret ID - return ret, parse(p.get("id"), &ret) -} - -func (p Params) GetFirstID(keys ...string) (ID, error) { - var ret ID +func (p Params) GetFirstID(keys ...string) (ids.IDV, error) { + var ret ids.IDV return ret, parse(p.getFirst(keys), &ret) } -func (p Params) GetOrID(key string, or ID) ID { - var ret ID +func (p Params) GetOrID(key string, or ids.IDV) ids.IDV { + var ret ids.IDV if err := parse(p.get(key), &ret); err == nil { return ret } return or } -func (p Params) GetFirstOrID(or ID, keys ...string) ID { - var ret ID +func (p Params) GetFirstOrID(or ids.IDV, keys ...string) ids.IDV { + var ret ids.IDV if err := parse(p.getFirst(keys), &ret); err == nil { return ret } return or } -// ** begin []ID {get, get first, get or, get first or} +// ** begin []ids.IDV {get, get first, get or, get first or} -func (p Params) GetIDList(key string) ([]ID, error) { - var ret []ID +func (p Params) GetIDList(key string) ([]ids.IDV, error) { + var ret []ids.IDV return ret, parse(p.get(key), &ret) } -func (p Params) GetFirstIDList(keys ...string) ([]ID, error) { - var ret []ID +func (p Params) GetFirstIDList(keys ...string) ([]ids.IDV, error) { + var ret []ids.IDV return ret, parse(p.getFirst(keys), &ret) } -func (p Params) GetOrIDList(key string, or []ID) []ID { - var ret []ID +func (p Params) GetOrIDList(key string, or []ids.IDV) []ids.IDV { + var ret []ids.IDV if err := parse(p.get(key), &ret); err == nil { return ret } return or } -func (p Params) GetFirstOrIDList(or []ID, keys ...string) []ID { - var ret []ID +func (p Params) GetFirstOrIDList(or []ids.IDV, keys ...string) []ids.IDV { + var ret []ids.IDV if err := parse(p.getFirst(keys), &ret); err == nil { return ret } diff --git a/server/ctrlsubsonic/params/params_test.go b/server/ctrlsubsonic/params/params_test.go deleted file mode 100644 index 1f23944..0000000 --- a/server/ctrlsubsonic/params/params_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package params - -import ( - "errors" - "testing" -) - -func TestParseID(t *testing.T) { - tcases := []struct { - param string - expType IDType - expValue int - expErr error - }{ - {param: "al-45", expType: IDTypeAlbum, expValue: 45}, - {param: "ar-2", expType: IDTypeArtist, expValue: 2}, - {param: "tr-43", expType: IDTypeTrack, expValue: 43}, - {param: "xx-1", expErr: ErrIDInvalid}, - {param: "al-howdy", expErr: ErrIDNotAnInt}, - } - for _, tcase := range tcases { - t.Run(tcase.param, func(t *testing.T) { - act, err := parseID(tcase.param) - if err != nil && !errors.Is(err, tcase.expErr) { - t.Fatalf("expected err %q, got %q", tcase.expErr, err) - } - if act.Value != tcase.expValue { - t.Errorf("expected value %d, got %d", tcase.expValue, act.Value) - } - if act.Type != tcase.expType { - t.Errorf("expected type %v, got %v", tcase.expType, act.Type) - } - }) - } -} - -// TODO? -func TestGet(t *testing.T) {} -func TestGetFirst(t *testing.T) {} -func TestGetOr(t *testing.T) {} -func TestGetList(t *testing.T) {} -func TestGetFirstList(t *testing.T) {} -func TestGetOrList(t *testing.T) {} -func TestGetInt(t *testing.T) {} -func TestGetFirstInt(t *testing.T) {} -func TestGetOrInt(t *testing.T) {} -func TestGetIntList(t *testing.T) {} -func TestGetFirstIntList(t *testing.T) {} -func TestGetOrIntList(t *testing.T) {} -func TestGetID(t *testing.T) {} -func TestGetFirstID(t *testing.T) {} -func TestGetOrID(t *testing.T) {} -func TestGetIDList(t *testing.T) {} -func TestGetFirstIDList(t *testing.T) {} -func TestGetOrIDList(t *testing.T) {} -func TestGetBool(t *testing.T) {} -func TestGetFirstBool(t *testing.T) {} -func TestGetOrBool(t *testing.T) {} -func TestGetBoolList(t *testing.T) {} -func TestGetFirstBoolList(t *testing.T) {} -func TestGetOrBoolList(t *testing.T) {} diff --git a/server/ctrlsubsonic/spec/construct_by_folder.go b/server/ctrlsubsonic/spec/construct_by_folder.go index 1eda95a..c439be3 100644 --- a/server/ctrlsubsonic/spec/construct_by_folder.go +++ b/server/ctrlsubsonic/spec/construct_by_folder.go @@ -3,15 +3,16 @@ package spec import ( "path" + "go.senan.xyz/gonic/server/ctrlsubsonic/params" "go.senan.xyz/gonic/server/db" ) func NewAlbumByFolder(f *db.Album) *Album { a := &Album{ Artist: f.Parent.RightPath, - ID: f.ID, + ID: params.IDAlbum(f.ID), IsDir: true, - ParentID: f.ParentID, + ParentID: params.IDAlbum(f.ParentID), Title: f.RightPath, TrackCount: f.ChildCount, } @@ -23,10 +24,10 @@ func NewAlbumByFolder(f *db.Album) *Album { func NewTCAlbumByFolder(f *db.Album) *TrackChild { trCh := &TrackChild{ - ID: f.ID, + ID: params.IDAlbum(f.ID), IsDir: true, Title: f.RightPath, - ParentID: f.ParentID, + ParentID: params.IDAlbum(f.ParentID), CreatedAt: f.UpdatedAt, } if f.Cover != "" { @@ -37,7 +38,7 @@ func NewTCAlbumByFolder(f *db.Album) *TrackChild { func NewTCTrackByFolder(t *db.Track, parent *db.Album) *TrackChild { trCh := &TrackChild{ - ID: t.ID, + ID: params.IDTrack(t.ID), ContentType: t.MIME(), Suffix: t.Ext(), Size: t.Size, @@ -50,7 +51,7 @@ func NewTCTrackByFolder(t *db.Track, parent *db.Album) *TrackChild { parent.RightPath, t.Filename, ), - ParentID: parent.ID, + ParentID: params.IDAlbum(parent.ID), Duration: t.Length, Bitrate: t.Bitrate, IsDir: false, @@ -67,8 +68,12 @@ func NewTCTrackByFolder(t *db.Track, parent *db.Album) *TrackChild { } func NewArtistByFolder(f *db.Album) *Artist { + // the db is structued around "browse by tags", and where + // an album is also a folder. so we're constructing an artist + // from an "album" where + // maybe TODO: rename the Album model to Folder return &Artist{ - ID: f.ID, + ID: params.IDAlbum(f.ID), Name: f.RightPath, AlbumCount: f.ChildCount, } diff --git a/server/ctrlsubsonic/spec/spec.go b/server/ctrlsubsonic/spec/spec.go index 2c7b50f..bb2359d 100644 --- a/server/ctrlsubsonic/spec/spec.go +++ b/server/ctrlsubsonic/spec/spec.go @@ -90,14 +90,14 @@ type Albums struct { type Album struct { // common - ID int `xml:"id,attr,omitempty" json:"id,string"` + ID string `xml:"id,attr,omitempty" json:"id"` CoverID int `xml:"coverArt,attr,omitempty" json:"coverArt,omitempty,string"` - ArtistID int `xml:"artistId,attr,omitempty" json:"artistId,omitempty,string"` + ArtistID string `xml:"artistId,attr,omitempty" json:"artistId,omitempty"` Artist string `xml:"artist,attr,omitempty" json:"artist,omitempty"` // browsing by folder (eg. getAlbumList) Title string `xml:"title,attr" json:"title"` Album string `xml:"album,attr" json:"album"` - ParentID int `xml:"parent,attr,omitempty" json:"parent,omitempty,string"` + ParentID string `xml:"parent,attr,omitempty" json:"parent,omitempty"` IsDir bool `xml:"isDir,attr,omitempty" json:"isDir,omitempty"` // browsing by tags (eg. getAlbumList2) Name string `xml:"name,attr" json:"name"` @@ -119,19 +119,19 @@ type TracksByGenre struct { type TrackChild struct { Album string `xml:"album,attr,omitempty" json:"album,omitempty"` - AlbumID int `xml:"albumId,attr,omitempty" json:"albumId,omitempty,string"` + AlbumID string `xml:"albumId,attr,omitempty" json:"albumId,omitempty"` Artist string `xml:"artist,attr,omitempty" json:"artist,omitempty"` - ArtistID int `xml:"artistId,attr,omitempty" json:"artistId,omitempty,string"` + ArtistID string `xml:"artistId,attr,omitempty" json:"artistId,omitempty"` Bitrate int `xml:"bitRate,attr,omitempty" json:"bitRate,omitempty"` ContentType string `xml:"contentType,attr,omitempty" json:"contentType,omitempty"` CoverID int `xml:"coverArt,attr,omitempty" json:"coverArt,omitempty,string"` CreatedAt time.Time `xml:"created,attr,omitempty" json:"created,omitempty"` Duration int `xml:"duration,attr,omitempty" json:"duration,omitempty"` Genre string `xml:"genre,attr,omitempty" json:"genre,omitempty"` - ID int `xml:"id,attr,omitempty" json:"id,omitempty,string"` + ID string `xml:"id,attr,omitempty" json:"id,omitempty"` IsDir bool `xml:"isDir,attr" json:"isDir"` IsVideo bool `xml:"isVideo,attr" json:"isVideo"` - ParentID int `xml:"parent,attr,omitempty" json:"parent,omitempty,string"` + ParentID string `xml:"parent,attr,omitempty" json:"parent,omitempty"` Path string `xml:"path,attr,omitempty" json:"path,omitempty"` Size int `xml:"size,attr,omitempty" json:"size,omitempty"` Suffix string `xml:"suffix,attr,omitempty" json:"suffix,omitempty"` @@ -147,7 +147,7 @@ type Artists struct { } type Artist struct { - ID int `xml:"id,attr,omitempty" json:"id,string"` + ID string `xml:"id,attr,omitempty" json:"id"` Name string `xml:"name,attr" json:"name"` CoverID int `xml:"coverArt,attr,omitempty" json:"coverArt,omitempty,string"` AlbumCount int `xml:"albumCount,attr" json:"albumCount"` @@ -166,8 +166,8 @@ type Index struct { } type Directory struct { - ID int `xml:"id,attr,omitempty" json:"id,string"` - ParentID int `xml:"parent,attr,omitempty" json:"parent,omitempty,string"` + ID string `xml:"id,attr,omitempty" json:"id"` + ParentID string `xml:"parent,attr,omitempty" json:"parent,omitempty"` Name string `xml:"name,attr,omitempty" json:"name"` Starred string `xml:"starred,attr,omitempty" json:"starred,omitempty"` Children []*TrackChild `xml:"child,omitempty" json:"child,omitempty"` @@ -178,7 +178,7 @@ type MusicFolders struct { } type MusicFolder struct { - ID int `xml:"id,attr,omitempty" json:"id,omitempty,string"` + ID string `xml:"id,attr,omitempty" json:"id,omitempty"` Name string `xml:"name,attr,omitempty" json:"name,omitempty"` } @@ -238,7 +238,7 @@ type Playlist struct { } type SimilarArtist struct { - ID int `xml:"id,attr" json:"id,string"` + ID string `xml:"id,attr" json:"id"` Name string `xml:"name,attr" json:"name"` AlbumCount int `xml:"albumCount,attr,omitempty" json:"albumCount,omitempty"` } diff --git a/server/ids/ids.go b/server/ids/ids.go new file mode 100644 index 0000000..1acd91d --- /dev/null +++ b/server/ids/ids.go @@ -0,0 +1,59 @@ +package ids + +// this package is at such a high level in the hierarchy because +// it's used by both `server/db` (for now) and `server/ctrlsubsonic` + +import ( + "errors" + "fmt" + "strconv" + "strings" +) + +var ( + ErrBadSeparator = errors.New("bad separator") + ErrNotAnInt = errors.New("not an int") + ErrBadPrefix = errors.New("bad prefix") +) + +type ID string + +const ( + // type values copied from subsonic + Artist ID = "ar" + Album ID = "al" + Track ID = "tr" +) + +var accepted = []ID{Artist, + Album, + Track, +} + +type IDV struct { + Type ID + Value int +} + +func (i IDV) String() string { + return fmt.Sprintf("%s-%d", i.Type, i.Value) +} + +func Parse(in string) (IDV, error) { + parts := strings.Split(in, "-") + if len(parts) != 2 { + return IDV{}, ErrBadSeparator + } + partType := parts[0] + partValue := parts[1] + val, err := strconv.Atoi(partValue) + if err != nil { + return IDV{}, fmt.Errorf("%q: %w", partValue, ErrNotAnInt) + } + for _, acc := range accepted { + if partType == string(acc) { + return IDV{Type: acc, Value: val}, nil + } + } + return IDV{}, fmt.Errorf("%q: %w", partType, ErrBadPrefix) +} diff --git a/server/ids/ids_test.go b/server/ids/ids_test.go new file mode 100644 index 0000000..6129c49 --- /dev/null +++ b/server/ids/ids_test.go @@ -0,0 +1,35 @@ +package ids + +import ( + "errors" + "testing" +) + +func TestParse(t *testing.T) { + tcases := []struct { + param string + expType ID + expValue int + expErr error + }{ + {param: "al-45", expType: Album, expValue: 45}, + {param: "ar-2", expType: Artist, expValue: 2}, + {param: "tr-43", expType: Track, expValue: 43}, + {param: "xx-1", expErr: ErrBadPrefix}, + {param: "al-howdy", expErr: ErrNotAnInt}, + } + for _, tcase := range tcases { + t.Run(tcase.param, func(t *testing.T) { + act, err := Parse(tcase.param) + if !errors.Is(err, tcase.expErr) { + t.Fatalf("expected err %q, got %q", tcase.expErr, err) + } + if act.Value != tcase.expValue { + t.Errorf("expected value %d, got %d", tcase.expValue, act.Value) + } + if act.Type != tcase.expType { + t.Errorf("expected type %v, got %v", tcase.expType, act.Type) + } + }) + } +}