refactor: validate limit only for artist downloads
This commit is contained in:
+1
-3
@@ -28,8 +28,6 @@ func newDownloadCmd() *cobra.Command {
|
|||||||
cmd.PersistentFlags().BoolVar(&opts.Genre, "genre", false, "fetch genre and add to file tags")
|
cmd.PersistentFlags().BoolVar(&opts.Genre, "genre", false, "fetch genre and add to file tags")
|
||||||
cmd.PersistentFlags().BoolVar(&opts.Strict, "strict", false, "fail the download if the requested quality is unavailable")
|
cmd.PersistentFlags().BoolVar(&opts.Strict, "strict", false, "fail the download if the requested quality is unavailable")
|
||||||
|
|
||||||
// Every subcommand shares opts: registering the artist-only --limit flag
|
|
||||||
// stores its default in the shared struct, which Validate requires for all kinds.
|
|
||||||
cmd.AddCommand(
|
cmd.AddCommand(
|
||||||
newDownloadSubCmd(deezer.KindAlbum, opts),
|
newDownloadSubCmd(deezer.KindAlbum, opts),
|
||||||
newDownloadSubCmd(deezer.KindPlaylist, opts),
|
newDownloadSubCmd(deezer.KindPlaylist, opts),
|
||||||
@@ -47,7 +45,7 @@ func newDownloadSubCmd(kind deezer.Kind, opts *download.Options) *cobra.Command
|
|||||||
Args: cobra.ExactArgs(1),
|
Args: cobra.ExactArgs(1),
|
||||||
PreRunE: func(cmd *cobra.Command, args []string) error {
|
PreRunE: func(cmd *cobra.Command, args []string) error {
|
||||||
opts.Quality = strings.ToLower(opts.Quality)
|
opts.Quality = strings.ToLower(opts.Quality)
|
||||||
return opts.Validate()
|
return opts.Validate(kind)
|
||||||
},
|
},
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
cfg, err := config.Load()
|
cfg, err := config.Load()
|
||||||
|
|||||||
@@ -3,6 +3,8 @@ package download
|
|||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/mathismqn/godeez/internal/deezer"
|
||||||
)
|
)
|
||||||
|
|
||||||
var validQualities = map[string]bool{
|
var validQualities = map[string]bool{
|
||||||
@@ -20,19 +22,21 @@ type Options struct {
|
|||||||
Strict bool
|
Strict bool
|
||||||
}
|
}
|
||||||
|
|
||||||
func (o *Options) Validate() error {
|
func (o *Options) Validate(kind deezer.Kind) error {
|
||||||
if !validQualities[o.Quality] {
|
if !validQualities[o.Quality] {
|
||||||
return fmt.Errorf("invalid quality option: %s", o.Quality)
|
return fmt.Errorf("invalid quality option: %s", o.Quality)
|
||||||
}
|
}
|
||||||
if o.Timeout <= 0 {
|
if o.Timeout <= 0 {
|
||||||
return fmt.Errorf("timeout must be a positive duration")
|
return fmt.Errorf("timeout must be a positive duration")
|
||||||
}
|
}
|
||||||
|
if kind == deezer.KindArtist {
|
||||||
if o.Limit <= 0 {
|
if o.Limit <= 0 {
|
||||||
return fmt.Errorf("limit must be a positive integer")
|
return fmt.Errorf("limit must be a positive integer")
|
||||||
}
|
}
|
||||||
if o.Limit > 100 {
|
if o.Limit > 100 {
|
||||||
return fmt.Errorf("limit must not exceed 100")
|
return fmt.Errorf("limit must not exceed 100")
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,6 +3,8 @@ package download
|
|||||||
import (
|
import (
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/mathismqn/godeez/internal/deezer"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestOptionsValidate(t *testing.T) {
|
func TestOptionsValidate(t *testing.T) {
|
||||||
@@ -11,18 +13,21 @@ func TestOptionsValidate(t *testing.T) {
|
|||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
mutate func(o *Options)
|
mutate func(o *Options)
|
||||||
|
kind deezer.Kind
|
||||||
wantErr bool
|
wantErr bool
|
||||||
}{
|
}{
|
||||||
{"valid", func(o *Options) {}, false},
|
{"valid", func(o *Options) {}, deezer.KindAlbum, false},
|
||||||
{"mp3_128", func(o *Options) { o.Quality = "mp3_128" }, false},
|
{"mp3_128", func(o *Options) { o.Quality = "mp3_128" }, deezer.KindAlbum, false},
|
||||||
{"flac", func(o *Options) { o.Quality = "flac" }, false},
|
{"flac", func(o *Options) { o.Quality = "flac" }, deezer.KindAlbum, false},
|
||||||
{"invalid quality", func(o *Options) { o.Quality = "ogg" }, true},
|
{"invalid quality", func(o *Options) { o.Quality = "ogg" }, deezer.KindAlbum, true},
|
||||||
{"uppercase quality", func(o *Options) { o.Quality = "MP3_320" }, true},
|
{"uppercase quality", func(o *Options) { o.Quality = "MP3_320" }, deezer.KindAlbum, true},
|
||||||
{"zero timeout", func(o *Options) { o.Timeout = 0 }, true},
|
{"zero timeout", func(o *Options) { o.Timeout = 0 }, deezer.KindAlbum, true},
|
||||||
{"negative timeout", func(o *Options) { o.Timeout = -time.Second }, true},
|
{"negative timeout", func(o *Options) { o.Timeout = -time.Second }, deezer.KindAlbum, true},
|
||||||
{"zero limit", func(o *Options) { o.Limit = 0 }, true},
|
{"artist zero limit", func(o *Options) { o.Limit = 0 }, deezer.KindArtist, true},
|
||||||
{"limit too high", func(o *Options) { o.Limit = 101 }, true},
|
{"artist limit too high", func(o *Options) { o.Limit = 101 }, deezer.KindArtist, true},
|
||||||
{"limit at max", func(o *Options) { o.Limit = 100 }, false},
|
{"artist limit at max", func(o *Options) { o.Limit = 100 }, deezer.KindArtist, false},
|
||||||
|
{"album ignores zero limit", func(o *Options) { o.Limit = 0 }, deezer.KindAlbum, false},
|
||||||
|
{"track ignores zero limit", func(o *Options) { o.Limit = 0 }, deezer.KindTrack, false},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
@@ -30,9 +35,9 @@ func TestOptionsValidate(t *testing.T) {
|
|||||||
opts := valid
|
opts := valid
|
||||||
tt.mutate(&opts)
|
tt.mutate(&opts)
|
||||||
|
|
||||||
err := opts.Validate()
|
err := opts.Validate(tt.kind)
|
||||||
if (err != nil) != tt.wantErr {
|
if (err != nil) != tt.wantErr {
|
||||||
t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr)
|
t.Errorf("Validate(%s) error = %v, wantErr %v", tt.kind, err, tt.wantErr)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user