From 90843374f694708a997c8e6e4d15142610c50cbf Mon Sep 17 00:00:00 2001 From: deepakgarg Date: Wed, 25 Mar 2020 21:43:39 -0700 Subject: [PATCH 1/4] #183 refactored the request options conversion code Now options are converted and saved before the request object is generated --- request.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/request.go b/request.go index b45cd38a..f5a50897 100644 --- a/request.go +++ b/request.go @@ -5,7 +5,7 @@ import ( "fmt" "reflect" - "github.com/ipfs/go-ipfs-files" + files "github.com/ipfs/go-ipfs-files" ) // Request represents a call to a command from a consumer @@ -24,7 +24,8 @@ type Request struct { // NewRequest returns a request initialized with given arguments // An non-nil error will be returned if the provided option values are invalid -func NewRequest(ctx context.Context, path []string, opts OptMap, args []string, file files.Directory, root *Command) (*Request, error) { +func NewRequest(ctx context.Context, path []string, opts OptMap, args []string, + file files.Directory, root *Command) (*Request, error) { if opts == nil { opts = make(OptMap) } @@ -34,6 +35,7 @@ func NewRequest(ctx context.Context, path []string, opts OptMap, args []string, return nil, err } + err = convertOptions(root, opts, path) req := &Request{ Path: path, Options: opts, @@ -44,7 +46,7 @@ func NewRequest(ctx context.Context, path []string, opts OptMap, args []string, Context: ctx, } - return req, req.convertOptions(root) + return req, err } // BodyArgs returns a scanner that returns arguments passed in the body as tokens. @@ -94,13 +96,13 @@ func (req *Request) SetOption(name string, value interface{}) { return } -func (req *Request) convertOptions(root *Command) error { - optDefs, err := root.GetOptions(req.Path) +func convertOptions(root *Command, opts OptMap, path []string) error { + optDefs, err := root.GetOptions(path) if err != nil { return err } - for k, v := range req.Options { + for k, v := range opts { opt, ok := optDefs[k] if !ok { continue @@ -118,7 +120,7 @@ func (req *Request) convertOptions(root *Command) error { return fmt.Errorf("Could not convert %s to type %q (for option %q)", value, opt.Type().String(), "-"+k) } - req.Options[k] = val + opts[k] = val } else { return fmt.Errorf("Option %q should be type %q, but got type %q", @@ -127,7 +129,7 @@ func (req *Request) convertOptions(root *Command) error { } for _, name := range opt.Names() { - if _, ok := req.Options[name]; name != k && ok { + if _, ok := opts[name]; name != k && ok { return fmt.Errorf("Duplicate command options were provided (%q and %q)", k, name) } From ee80c8ee32bf5f8ab6e544d4c973db258f260357 Mon Sep 17 00:00:00 2001 From: deepakgarg Date: Wed, 25 Mar 2020 21:43:39 -0700 Subject: [PATCH 2/4] Fixes #183 Now options are converted and saved before the request object is generated --- request.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/request.go b/request.go index b45cd38a..547f545a 100644 --- a/request.go +++ b/request.go @@ -5,7 +5,7 @@ import ( "fmt" "reflect" - "github.com/ipfs/go-ipfs-files" + files "github.com/ipfs/go-ipfs-files" ) // Request represents a call to a command from a consumer @@ -24,7 +24,12 @@ type Request struct { // NewRequest returns a request initialized with given arguments // An non-nil error will be returned if the provided option values are invalid -func NewRequest(ctx context.Context, path []string, opts OptMap, args []string, file files.Directory, root *Command) (*Request, error) { +func NewRequest(ctx context.Context, + path []string, opts OptMap, + args []string, + file files.Directory, + root *Command, +) (*Request, error) { if opts == nil { opts = make(OptMap) } @@ -34,6 +39,7 @@ func NewRequest(ctx context.Context, path []string, opts OptMap, args []string, return nil, err } + err = checkAndConvertOptions(root, opts, path) req := &Request{ Path: path, Options: opts, @@ -44,7 +50,7 @@ func NewRequest(ctx context.Context, path []string, opts OptMap, args []string, Context: ctx, } - return req, req.convertOptions(root) + return req, err } // BodyArgs returns a scanner that returns arguments passed in the body as tokens. @@ -94,13 +100,13 @@ func (req *Request) SetOption(name string, value interface{}) { return } -func (req *Request) convertOptions(root *Command) error { - optDefs, err := root.GetOptions(req.Path) +func checkAndConvertOptions(root *Command, opts OptMap, path []string) error { + optDefs, err := root.GetOptions(path) if err != nil { return err } - for k, v := range req.Options { + for k, v := range opts { opt, ok := optDefs[k] if !ok { continue @@ -118,7 +124,7 @@ func (req *Request) convertOptions(root *Command) error { return fmt.Errorf("Could not convert %s to type %q (for option %q)", value, opt.Type().String(), "-"+k) } - req.Options[k] = val + opts[k] = val } else { return fmt.Errorf("Option %q should be type %q, but got type %q", @@ -127,7 +133,7 @@ func (req *Request) convertOptions(root *Command) error { } for _, name := range opt.Names() { - if _, ok := req.Options[name]; name != k && ok { + if _, ok := opts[name]; name != k && ok { return fmt.Errorf("Duplicate command options were provided (%q and %q)", k, name) } From 1b028fbf0bb11e32e437d82ac1a8d9ee1e20e4a2 Mon Sep 17 00:00:00 2001 From: deepakgarg Date: Thu, 26 Mar 2020 19:05:18 -0700 Subject: [PATCH 3/4] Fixes #183. Per PR comments, not modifying the mutating opts object --- request.go | 43 +++++++++++++++++-------------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/request.go b/request.go index 35c3b908..c418b6a5 100644 --- a/request.go +++ b/request.go @@ -24,34 +24,24 @@ type Request struct { // NewRequest returns a request initialized with given arguments // An non-nil error will be returned if the provided option values are invalid -<<<<<<< HEAD func NewRequest(ctx context.Context, - path []string, opts OptMap, + path []string, + opts OptMap, args []string, file files.Directory, root *Command, ) (*Request, error) { -======= -func NewRequest(ctx context.Context, path []string, opts OptMap, args []string, - file files.Directory, root *Command) (*Request, error) { ->>>>>>> 90843374f694708a997c8e6e4d15142610c50cbf - if opts == nil { - opts = make(OptMap) - } cmd, err := root.Get(path) if err != nil { return nil, err } -<<<<<<< HEAD - err = checkAndConvertOptions(root, opts, path) -======= - err = convertOptions(root, opts, path) ->>>>>>> 90843374f694708a997c8e6e4d15142610c50cbf + options, err := checkAndConvertOptions(root, opts, path) + req := &Request{ Path: path, - Options: opts, + Options: options, Arguments: args, Files: file, Root: root, @@ -109,14 +99,15 @@ func (req *Request) SetOption(name string, value interface{}) { return } -<<<<<<< HEAD -func checkAndConvertOptions(root *Command, opts OptMap, path []string) error { -======= -func convertOptions(root *Command, opts OptMap, path []string) error { ->>>>>>> 90843374f694708a997c8e6e4d15142610c50cbf +func checkAndConvertOptions(root *Command, opts OptMap, path []string) (OptMap, error) { optDefs, err := root.GetOptions(path) + options := make(OptMap) + if err != nil { - return err + return options, err + } + for k, v := range opts { + options[k] = v } for k, v := range opts { @@ -134,26 +125,26 @@ func convertOptions(root *Command, opts OptMap, path []string) error { if len(str) == 0 { value = "empty value" } - return fmt.Errorf("Could not convert %s to type %q (for option %q)", + return options, fmt.Errorf("Could not convert %s to type %q (for option %q)", value, opt.Type().String(), "-"+k) } - opts[k] = val + options[k] = val } else { - return fmt.Errorf("Option %q should be type %q, but got type %q", + return options, fmt.Errorf("Option %q should be type %q, but got type %q", k, opt.Type().String(), kind.String()) } } for _, name := range opt.Names() { if _, ok := opts[name]; name != k && ok { - return fmt.Errorf("Duplicate command options were provided (%q and %q)", + return options, fmt.Errorf("Duplicate command options were provided (%q and %q)", k, name) } } } - return nil + return options, nil } // GetEncoding returns the EncodingType set in a request, falling back to JSON From c9accceab3600c49eb9b0ae65a47939f78d23c0b Mon Sep 17 00:00:00 2001 From: deepakgarg Date: Thu, 26 Mar 2020 19:30:12 -0700 Subject: [PATCH 4/4] Fixes #183. Not modifying the mutating opts object per comments --- request.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/request.go b/request.go index c418b6a5..b12f9458 100644 --- a/request.go +++ b/request.go @@ -137,7 +137,7 @@ func checkAndConvertOptions(root *Command, opts OptMap, path []string) (OptMap, } for _, name := range opt.Names() { - if _, ok := opts[name]; name != k && ok { + if _, ok := options[name]; name != k && ok { return options, fmt.Errorf("Duplicate command options were provided (%q and %q)", k, name) }