From dd2a10509f3dc548d13dacfe9c0f93a81f507ffc Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 8 Oct 2014 14:27:36 -0700 Subject: [PATCH 01/73] commands: Implemented Command --- commands/command.go | 85 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 commands/command.go diff --git a/commands/command.go b/commands/command.go new file mode 100644 index 00000000000..d118d780ea7 --- /dev/null +++ b/commands/command.go @@ -0,0 +1,85 @@ +package commands + +import ( + "fmt" + "strings" + "reflect" +) + +type Command struct { + Help string + Options []Option + f func(*Request) (interface{}, error) + subcommands map[string]*Command +} + +// Register adds a subcommand +func (c *Command) Register(id string, sub *Command) error { + if c.subcommands == nil { + c.subcommands = make(map[string]*Command) + } + + // TODO: check for duplicate option names + + if _, ok := c.subcommands[id]; ok { + return fmt.Errorf("There is already a subcommand registered with id '%s'", id) + } + + c.subcommands[id] = sub + return nil +} + +// Call invokes the command at the given subcommand path +func (c *Command) Call(path []string, req *Request) (interface{}, error) { + options := make([]Option, len(c.Options)) + copy(options, c.Options) + cmd := c + + if path != nil { + for i, id := range path { + cmd = c.Sub(id) + + if cmd == nil { + pathS := strings.Join(path[0:i], "/") + return nil, fmt.Errorf("Undefined command: '%s'", pathS) + } + + options = append(options, cmd.Options...) + } + } + + optionsMap := make(map[string]Option) + for _, opt := range options { + for _, name := range opt.Names { + optionsMap[name] = opt + } + } + + for k, v := range req.options { + opt, ok := optionsMap[k] + + if !ok { + return nil, fmt.Errorf("Unrecognized command option: '%s'", k) + } + + for _, name := range opt.Names { + if _, ok = req.options[name]; name != k && ok { + return nil, fmt.Errorf("Duplicate command options were provided ('%s' and '%s')", + k, name) + } + } + + kind := reflect.TypeOf(v).Kind() + if kind != opt.Type { + return nil, fmt.Errorf("Option '%s' should be type '%s', but got type '%s'", + k, opt.Type.String(), kind.String()) + } + } + + return cmd.f(req) +} + +// Sub returns the subcommand with the given id +func (c *Command) Sub(id string) *Command { + return c.subcommands[id] +} From 30ea427b7b8a68e66f3bbda7c1c926e1dc0b7a1f Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 8 Oct 2014 14:28:39 -0700 Subject: [PATCH 02/73] commands: Created Option struct --- commands/option.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 commands/option.go diff --git a/commands/option.go b/commands/option.go new file mode 100644 index 00000000000..ca26208b8c2 --- /dev/null +++ b/commands/option.go @@ -0,0 +1,20 @@ +package commands + +import "reflect" + +const ( + Invalid = reflect.Invalid + Bool = reflect.Bool + Int = reflect.Int + Uint = reflect.Uint + Float = reflect.Float32 + String = reflect.String +) + +// Option is used to specify a field that will be provided by a consumer +type Option struct { + Names []string // a list of unique names to + Type reflect.Kind // value must be this type + //Default interface{} // the default value (ignored if `Required` is true) + //Required bool // whether or not the option must be provided +} From 15b7388c0e5fc9c6b8d42ea00d0ac209fc82262f Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 8 Oct 2014 14:29:17 -0700 Subject: [PATCH 03/73] commands: Request struct --- commands/request.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 commands/request.go diff --git a/commands/request.go b/commands/request.go new file mode 100644 index 00000000000..956456ea3e4 --- /dev/null +++ b/commands/request.go @@ -0,0 +1,20 @@ +package commands + +// Request represents a call to a command from a consumer +type Request struct { + options map[string]interface{} +} + +/*func (r *Request) Option(name string) interface{} { + +} + +func (r *Request) Arguments() interface{} { + +}*/ + +func NewRequest() *Request { + return &Request{ + make(map[string]interface{}), + } +} From dd682963a2f14535b77ce78c243f83052122eaaf Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 8 Oct 2014 14:30:27 -0700 Subject: [PATCH 04/73] commands: Wrote tests for command option validation --- commands/command_test.go | 60 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 commands/command_test.go diff --git a/commands/command_test.go b/commands/command_test.go new file mode 100644 index 00000000000..f08e3756024 --- /dev/null +++ b/commands/command_test.go @@ -0,0 +1,60 @@ +package commands + +import "testing" + +func TestOptionValidation(t *testing.T) { + cmd := Command{ + Options: []Option{ + Option{ []string{ "b", "beep" }, Int }, + Option{ []string{ "B", "boop" }, String }, + }, + f: func(req *Request) (interface{}, error) { + return nil, nil + }, + } + + req := NewRequest() + req.options["foo"] = 5 + _, err := cmd.Call(nil, req) + if err == nil { + t.Error("Should have failed (unrecognized command)") + } + + req = NewRequest() + req.options["beep"] = 5 + req.options["b"] = 10 + _, err = cmd.Call(nil, req) + if err == nil { + t.Error("Should have failed (duplicate options)") + } + + req = NewRequest() + req.options["beep"] = "foo" + _, err = cmd.Call(nil, req) + if err == nil { + t.Error("Should have failed (incorrect type)") + } + + req = NewRequest() + req.options["beep"] = 5 + _, err = cmd.Call(nil, req) + if err != nil { + t.Error("Should have passed") + } + + req = NewRequest() + req.options["beep"] = 5 + req.options["boop"] = "test" + _, err = cmd.Call(nil, req) + if err != nil { + t.Error("Should have passed") + } + + req = NewRequest() + req.options["b"] = 5 + req.options["B"] = "test" + _, err = cmd.Call(nil, req) + if err != nil { + t.Error("Should have passed") + } +} From 5b18844c063b9dc344046a98f30e5f70d05aee0c Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Thu, 9 Oct 2014 13:39:13 -0700 Subject: [PATCH 05/73] commands: Check for option name collisions --- commands/command.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/commands/command.go b/commands/command.go index d118d780ea7..66439af9ee3 100644 --- a/commands/command.go +++ b/commands/command.go @@ -19,7 +19,13 @@ func (c *Command) Register(id string, sub *Command) error { c.subcommands = make(map[string]*Command) } - // TODO: check for duplicate option names + // check for duplicate option names (only checks downwards) + names := make(map[string]bool) + c.checkOptions(names) + err := sub.checkOptions(names) + if err != nil { + return err + } if _, ok := c.subcommands[id]; ok { return fmt.Errorf("There is already a subcommand registered with id '%s'", id) @@ -83,3 +89,23 @@ func (c *Command) Call(path []string, req *Request) (interface{}, error) { func (c *Command) Sub(id string) *Command { return c.subcommands[id] } + +func (c *Command) checkOptions(names map[string]bool) error { + for _, opt := range c.Options { + for _, name := range opt.Names { + if _, ok := names[name]; ok { + return fmt.Errorf("Multiple options are using the same name ('%s')", name) + } + names[name] = true + } + } + + for _, cmd := range c.subcommands { + err := cmd.checkOptions(names) + if err != nil { + return err + } + } + + return nil +} From e593c180fee8ddefac8520f31cd8429d308e1dd9 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Thu, 9 Oct 2014 13:39:34 -0700 Subject: [PATCH 06/73] commands: Added tests for Command.Register --- commands/command_test.go | 55 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/commands/command_test.go b/commands/command_test.go index f08e3756024..d0ce34147bf 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -58,3 +58,58 @@ func TestOptionValidation(t *testing.T) { t.Error("Should have passed") } } + +func TestRegistration(t *testing.T) { + cmds := []*Command{ + &Command{ + Options: []Option{ + Option{ []string{ "beep" }, Int }, + }, + f: func(req *Request) (interface{}, error) { + return nil, nil + }, + }, + + &Command{ + Options: []Option{ + Option{ []string{ "boop" }, Int }, + }, + f: func(req *Request) (interface{}, error) { + return nil, nil + }, + }, + + &Command{ + Options: []Option{ + Option{ []string{ "boop" }, String }, + }, + f: func(req *Request) (interface{}, error) { + return nil, nil + }, + }, + + &Command{ + Options: []Option{ + Option{ []string{ "bop" }, String }, + }, + f: func(req *Request) (interface{}, error) { + return nil, nil + }, + }, + } + + err := cmds[0].Register("foo", cmds[1]) + if err != nil { + t.Error("Should have passed") + } + + err = cmds[0].Register("bar", cmds[2]) + if err == nil { + t.Error("Should have failed (option name collision)") + } + + err = cmds[0].Register("foo", cmds[3]) + if err == nil { + t.Error("Should have failed (subcommand name collision)") + } +} From 47ebf174f4e726346a651bc74ee234bbaeb6cf17 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 11:40:57 -0700 Subject: [PATCH 07/73] commands: Created a list of global options (for options owned by commands package) --- commands/option.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/commands/option.go b/commands/option.go index ca26208b8c2..e14b1bf68ec 100644 --- a/commands/option.go +++ b/commands/option.go @@ -15,6 +15,17 @@ const ( type Option struct { Names []string // a list of unique names to Type reflect.Kind // value must be this type + + // TODO: add more features(?): //Default interface{} // the default value (ignored if `Required` is true) //Required bool // whether or not the option must be provided } + +// options that are used by this package +var globalOptions []Option = []Option{ + Option{ []string{ "enc", "encoding" }, String }, +} +// the above array of Options, wrapped in a Command +var globalCommand *Command = &Command{ + Options: globalOptions, +} From d7e9afc801a8160b50d5642685c663ddd47e7822 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 11:41:43 -0700 Subject: [PATCH 08/73] commands: Use global options when registering and calling commands --- commands/command.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/commands/command.go b/commands/command.go index 66439af9ee3..0b35d326a59 100644 --- a/commands/command.go +++ b/commands/command.go @@ -21,6 +21,7 @@ func (c *Command) Register(id string, sub *Command) error { // check for duplicate option names (only checks downwards) names := make(map[string]bool) + globalCommand.checkOptions(names) c.checkOptions(names) err := sub.checkOptions(names) if err != nil { @@ -39,6 +40,7 @@ func (c *Command) Register(id string, sub *Command) error { func (c *Command) Call(path []string, req *Request) (interface{}, error) { options := make([]Option, len(c.Options)) copy(options, c.Options) + options = append(options, globalOptions...) cmd := c if path != nil { From 97ce60f8a4843eb660457747237e5ad768dc0dd0 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 11:42:01 -0700 Subject: [PATCH 09/73] commands: Added global options list to command tests --- commands/command_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/commands/command_test.go b/commands/command_test.go index d0ce34147bf..bb4fc3a497a 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -57,6 +57,13 @@ func TestOptionValidation(t *testing.T) { if err != nil { t.Error("Should have passed") } + + req = NewRequest() + req.options["enc"] = "json" + _, err = cmd.Call(nil, req) + if err != nil { + t.Error("Should have passed") + } } func TestRegistration(t *testing.T) { @@ -96,6 +103,15 @@ func TestRegistration(t *testing.T) { return nil, nil }, }, + + &Command{ + Options: []Option{ + Option{ []string{ "enc" }, String }, + }, + f: func(req *Request) (interface{}, error) { + return nil, nil + }, + }, } err := cmds[0].Register("foo", cmds[1]) @@ -112,4 +128,9 @@ func TestRegistration(t *testing.T) { if err == nil { t.Error("Should have failed (subcommand name collision)") } + + err = cmds[0].Register("baz", cmds[4]) + if err == nil { + t.Error("Should have failed (option name collision with global options)") + } } From f31fd53df9731e14b31c6986ef074ad94ee3bba1 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 12:36:15 -0700 Subject: [PATCH 10/73] commands: Added Response --- commands/response.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 commands/response.go diff --git a/commands/response.go b/commands/response.go new file mode 100644 index 00000000000..6a60f143582 --- /dev/null +++ b/commands/response.go @@ -0,0 +1,24 @@ +package commands + +type ErrorType uint +const ( + Normal ErrorType = iota // general errors + Client // error was caused by the client, (e.g. invalid CLI usage) + // TODO: add more types of errors for better error-specific handling +) + +type Response struct { + req *Request + Error error + ErrorType ErrorType + Value interface{} +} + +func (r *Response) SetError(err error, errType ErrorType) { + r.Error = err + r.ErrorType = errType +} + +/*func (r *Response) Encode() ([]byte, error) { + +}*/ From b2ee05a0becfbf6a4096339c4281a6b117b4fe94 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 12:36:55 -0700 Subject: [PATCH 11/73] commands: Updated Command to use Response for output rather than (interface{}, error) --- commands/command.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/commands/command.go b/commands/command.go index 0b35d326a59..96d79214fb7 100644 --- a/commands/command.go +++ b/commands/command.go @@ -9,7 +9,7 @@ import ( type Command struct { Help string Options []Option - f func(*Request) (interface{}, error) + f func(*Request, *Response) subcommands map[string]*Command } @@ -37,11 +37,12 @@ func (c *Command) Register(id string, sub *Command) error { } // Call invokes the command at the given subcommand path -func (c *Command) Call(path []string, req *Request) (interface{}, error) { +func (c *Command) Call(path []string, req *Request) *Response { options := make([]Option, len(c.Options)) copy(options, c.Options) options = append(options, globalOptions...) cmd := c + res := &Response{ req: req } if path != nil { for i, id := range path { @@ -49,7 +50,8 @@ func (c *Command) Call(path []string, req *Request) (interface{}, error) { if cmd == nil { pathS := strings.Join(path[0:i], "/") - return nil, fmt.Errorf("Undefined command: '%s'", pathS) + res.SetError(fmt.Errorf("Undefined command: '%s'", pathS), Client) + return res } options = append(options, cmd.Options...) @@ -67,24 +69,29 @@ func (c *Command) Call(path []string, req *Request) (interface{}, error) { opt, ok := optionsMap[k] if !ok { - return nil, fmt.Errorf("Unrecognized command option: '%s'", k) + res.SetError(fmt.Errorf("Unrecognized command option: '%s'", k), Client) + return res } for _, name := range opt.Names { if _, ok = req.options[name]; name != k && ok { - return nil, fmt.Errorf("Duplicate command options were provided ('%s' and '%s')", - k, name) + res.SetError(fmt.Errorf("Duplicate command options were provided ('%s' and '%s')", + k, name), Client) + return res } } kind := reflect.TypeOf(v).Kind() if kind != opt.Type { - return nil, fmt.Errorf("Option '%s' should be type '%s', but got type '%s'", - k, opt.Type.String(), kind.String()) + res.SetError(fmt.Errorf("Option '%s' should be type '%s', but got type '%s'", + k, opt.Type.String(), kind.String()), Client) + return res } } - return cmd.f(req) + cmd.f(req, res) + + return res } // Sub returns the subcommand with the given id From df034c9c0be29c4e0dc875a06b63133140c4448d Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 12:37:11 -0700 Subject: [PATCH 12/73] commands: Updated Command tests for new Response API --- commands/command_test.go | 52 ++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/commands/command_test.go b/commands/command_test.go index bb4fc3a497a..95667afee32 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -8,60 +8,58 @@ func TestOptionValidation(t *testing.T) { Option{ []string{ "b", "beep" }, Int }, Option{ []string{ "B", "boop" }, String }, }, - f: func(req *Request) (interface{}, error) { - return nil, nil - }, + f: func(req *Request, res *Response) {}, } req := NewRequest() req.options["foo"] = 5 - _, err := cmd.Call(nil, req) - if err == nil { + res := cmd.Call(nil, req) + if res.Error == nil { t.Error("Should have failed (unrecognized command)") } req = NewRequest() req.options["beep"] = 5 req.options["b"] = 10 - _, err = cmd.Call(nil, req) - if err == nil { + res = cmd.Call(nil, req) + if res.Error == nil { t.Error("Should have failed (duplicate options)") } req = NewRequest() req.options["beep"] = "foo" - _, err = cmd.Call(nil, req) - if err == nil { + res = cmd.Call(nil, req) + if res.Error == nil { t.Error("Should have failed (incorrect type)") } req = NewRequest() req.options["beep"] = 5 - _, err = cmd.Call(nil, req) - if err != nil { + res = cmd.Call(nil, req) + if res.Error != nil { t.Error("Should have passed") } req = NewRequest() req.options["beep"] = 5 req.options["boop"] = "test" - _, err = cmd.Call(nil, req) - if err != nil { + res = cmd.Call(nil, req) + if res.Error != nil { t.Error("Should have passed") } req = NewRequest() req.options["b"] = 5 req.options["B"] = "test" - _, err = cmd.Call(nil, req) - if err != nil { + res = cmd.Call(nil, req) + if res.Error != nil { t.Error("Should have passed") } req = NewRequest() req.options["enc"] = "json" - _, err = cmd.Call(nil, req) - if err != nil { + res = cmd.Call(nil, req) + if res.Error != nil { t.Error("Should have passed") } } @@ -72,45 +70,35 @@ func TestRegistration(t *testing.T) { Options: []Option{ Option{ []string{ "beep" }, Int }, }, - f: func(req *Request) (interface{}, error) { - return nil, nil - }, + f: func(req *Request, res *Response) {}, }, &Command{ Options: []Option{ Option{ []string{ "boop" }, Int }, }, - f: func(req *Request) (interface{}, error) { - return nil, nil - }, + f: func(req *Request, res *Response) {}, }, &Command{ Options: []Option{ Option{ []string{ "boop" }, String }, }, - f: func(req *Request) (interface{}, error) { - return nil, nil - }, + f: func(req *Request, res *Response) {}, }, &Command{ Options: []Option{ Option{ []string{ "bop" }, String }, }, - f: func(req *Request) (interface{}, error) { - return nil, nil - }, + f: func(req *Request, res *Response) {}, }, &Command{ Options: []Option{ Option{ []string{ "enc" }, String }, }, - f: func(req *Request) (interface{}, error) { - return nil, nil - }, + f: func(req *Request, res *Response) {}, }, } From d1595ce34cb388db250ab69a28078a31505f0a70 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 12:38:03 -0700 Subject: [PATCH 13/73] commands: Added basic methods to Request --- commands/request.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/commands/request.go b/commands/request.go index 956456ea3e4..824d48d71e7 100644 --- a/commands/request.go +++ b/commands/request.go @@ -3,18 +3,20 @@ package commands // Request represents a call to a command from a consumer type Request struct { options map[string]interface{} + arguments []string } -/*func (r *Request) Option(name string) interface{} { - +func (r *Request) Option(name string) interface{} { + return r.options[name] } -func (r *Request) Arguments() interface{} { - -}*/ +func (r *Request) Arguments() []string { + return r.arguments +} func NewRequest() *Request { return &Request{ make(map[string]interface{}), + make([]string, 0), } } From 95b0dd29f84cee80f4fcc139f7997817490de692 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 12:54:04 -0700 Subject: [PATCH 14/73] commands: Added an Error struct for creating marshallable errors --- commands/response.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/commands/response.go b/commands/response.go index 6a60f143582..a538814c59f 100644 --- a/commands/response.go +++ b/commands/response.go @@ -7,6 +7,12 @@ const ( // TODO: add more types of errors for better error-specific handling ) +// Error is a struct for marshalling errors +type Error struct { + message string + code ErrorType +} + type Response struct { req *Request Error error @@ -19,6 +25,10 @@ func (r *Response) SetError(err error, errType ErrorType) { r.ErrorType = errType } +func (r *Response) FormatError() Error { + return Error{ r.Error.Error(), r.ErrorType } +} + /*func (r *Response) Encode() ([]byte, error) { }*/ From a3a843759b4ed6db41ac8b2c316bc27938b0e11d Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 13:33:47 -0700 Subject: [PATCH 15/73] commands: Added marshalling to Response --- commands/response.go | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/commands/response.go b/commands/response.go index a538814c59f..85ce76fee01 100644 --- a/commands/response.go +++ b/commands/response.go @@ -1,5 +1,12 @@ package commands +import ( + "fmt" + "strings" + "encoding/json" + "encoding/xml" +) + type ErrorType uint const ( Normal ErrorType = iota // general errors @@ -13,6 +20,19 @@ type Error struct { code ErrorType } +type EncodingType string +const ( + Json = "json" + Xml = "xml" + // TODO: support more encoding types +) + +type Marshaller func(v interface{})([]byte, error) +var marshallers = map[EncodingType]Marshaller{ + Json: json.Marshal, + Xml: xml.Marshal, +} + type Response struct { req *Request Error error @@ -29,6 +49,17 @@ func (r *Response) FormatError() Error { return Error{ r.Error.Error(), r.ErrorType } } -/*func (r *Response) Encode() ([]byte, error) { +func (r *Response) Marshal() ([]byte, error) { + enc := r.req.Option("enc") + if enc == nil { + return nil, fmt.Errorf("No encoding type was specified") + } + encType := EncodingType(strings.ToLower(enc.(string))) -}*/ + marshaller, ok := marshallers[encType] + if !ok { + return nil, fmt.Errorf("No marshaller found for encoding type '%s'", enc) + } + + return marshaller(r.Value) +} From 808d9c19122f0ec2708615720b43b55222e11994 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 13:34:03 -0700 Subject: [PATCH 16/73] commands: Wrote tests for Response marshalling --- commands/response_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 commands/response_test.go diff --git a/commands/response_test.go b/commands/response_test.go new file mode 100644 index 00000000000..612667fa06b --- /dev/null +++ b/commands/response_test.go @@ -0,0 +1,32 @@ +package commands + +import "testing" + +type TestOutput struct { + Foo, Bar string + Baz int +} + +func TestMarshalling(t *testing.T) { + req := NewRequest() + + res := Response{ + req: req, + Value: TestOutput{ "beep", "boop", 1337 }, + } + + _, err := res.Marshal() + if err == nil { + t.Error("Should have failed (no encoding type specified in request)") + } + + req.options["enc"] = Json + bytes, err := res.Marshal() + if err != nil { + t.Error("Should have passed") + } + output := string(bytes) + if output != "{\"Foo\":\"beep\",\"Bar\":\"boop\",\"Baz\":1337}" { + t.Error("Incorrect JSON output") + } +} From 308ee5c9495bbb1afa1affe1c700f7d9d53e7d29 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 13:49:12 -0700 Subject: [PATCH 17/73] commands: Added Request#SetOption so we can set options with multiple names --- commands/request.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/commands/request.go b/commands/request.go index 824d48d71e7..7373a1a243a 100644 --- a/commands/request.go +++ b/commands/request.go @@ -10,6 +10,14 @@ func (r *Request) Option(name string) interface{} { return r.options[name] } +func (r *Request) SetOption(option Option, value interface{}) { + // saves the option value in the map, indexed by each name + // (so commands can retrieve it using any of the names) + for _, name := range option.Names { + r.options[name] = value + } +} + func (r *Request) Arguments() []string { return r.arguments } From 01938ac57463f74bf1ec8c242530190542c0f31e Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 13:49:42 -0700 Subject: [PATCH 18/73] commands: Updated Response test to use safer option setting --- commands/response_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/response_test.go b/commands/response_test.go index 612667fa06b..a7b1a795d68 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -20,7 +20,7 @@ func TestMarshalling(t *testing.T) { t.Error("Should have failed (no encoding type specified in request)") } - req.options["enc"] = Json + req.SetOption(globalOptions[0], Json) bytes, err := res.Marshal() if err != nil { t.Error("Should have passed") From aa592cec8013edba9c4aeae77028acc2732dc02c Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 13:59:40 -0700 Subject: [PATCH 19/73] commands: Added error marshalling to Response --- commands/response.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/commands/response.go b/commands/response.go index 85ce76fee01..07e91d12ff2 100644 --- a/commands/response.go +++ b/commands/response.go @@ -16,8 +16,8 @@ const ( // Error is a struct for marshalling errors type Error struct { - message string - code ErrorType + Message string + Code ErrorType } type EncodingType string @@ -50,6 +50,10 @@ func (r *Response) FormatError() Error { } func (r *Response) Marshal() ([]byte, error) { + if r.Error == nil && r.Value == nil { + return nil, fmt.Errorf("No error or value set, there is nothing to marshal") + } + enc := r.req.Option("enc") if enc == nil { return nil, fmt.Errorf("No encoding type was specified") @@ -61,5 +65,10 @@ func (r *Response) Marshal() ([]byte, error) { return nil, fmt.Errorf("No marshaller found for encoding type '%s'", enc) } - return marshaller(r.Value) + if r.Error != nil { + err := r.FormatError() + return marshaller(err) + } else { + return marshaller(r.Value) + } } From 94ca2642a13af96a04f8c2e2cc0eec72676efda3 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 14:00:02 -0700 Subject: [PATCH 20/73] commands: Added test for Response error marshalling --- commands/response_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/commands/response_test.go b/commands/response_test.go index a7b1a795d68..e1a96676d23 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -1,6 +1,9 @@ package commands -import "testing" +import ( + "testing" + "fmt" +) type TestOutput struct { Foo, Bar string @@ -29,4 +32,14 @@ func TestMarshalling(t *testing.T) { if output != "{\"Foo\":\"beep\",\"Bar\":\"boop\",\"Baz\":1337}" { t.Error("Incorrect JSON output") } + + res.SetError(fmt.Errorf("You broke something!"), Client) + bytes, err = res.Marshal() + if err != nil { + t.Error("Should have passed") + } + output = string(bytes) + if output != "{\"Message\":\"You broke something!\",\"Code\":1}" { + t.Error("Incorrect JSON output") + } } From 43670971d802d7d381c87aeb61b22ff84fd5453f Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 10 Oct 2014 14:24:36 -0700 Subject: [PATCH 21/73] commands: Formatted code --- commands/command.go | 196 ++++++++++++++++---------------- commands/command_test.go | 230 +++++++++++++++++++------------------- commands/option.go | 27 ++--- commands/request.go | 26 ++--- commands/response.go | 83 +++++++------- commands/response_test.go | 62 +++++----- 6 files changed, 314 insertions(+), 310 deletions(-) diff --git a/commands/command.go b/commands/command.go index 96d79214fb7..a98b1989a5c 100644 --- a/commands/command.go +++ b/commands/command.go @@ -1,120 +1,120 @@ package commands import ( - "fmt" - "strings" - "reflect" + "fmt" + "reflect" + "strings" ) type Command struct { - Help string - Options []Option - f func(*Request, *Response) - subcommands map[string]*Command + Help string + Options []Option + f func(*Request, *Response) + subcommands map[string]*Command } // Register adds a subcommand func (c *Command) Register(id string, sub *Command) error { - if c.subcommands == nil { - c.subcommands = make(map[string]*Command) - } - - // check for duplicate option names (only checks downwards) - names := make(map[string]bool) - globalCommand.checkOptions(names) - c.checkOptions(names) - err := sub.checkOptions(names) - if err != nil { - return err - } - - if _, ok := c.subcommands[id]; ok { - return fmt.Errorf("There is already a subcommand registered with id '%s'", id) - } - - c.subcommands[id] = sub - return nil + if c.subcommands == nil { + c.subcommands = make(map[string]*Command) + } + + // check for duplicate option names (only checks downwards) + names := make(map[string]bool) + globalCommand.checkOptions(names) + c.checkOptions(names) + err := sub.checkOptions(names) + if err != nil { + return err + } + + if _, ok := c.subcommands[id]; ok { + return fmt.Errorf("There is already a subcommand registered with id '%s'", id) + } + + c.subcommands[id] = sub + return nil } // Call invokes the command at the given subcommand path func (c *Command) Call(path []string, req *Request) *Response { - options := make([]Option, len(c.Options)) - copy(options, c.Options) - options = append(options, globalOptions...) - cmd := c - res := &Response{ req: req } - - if path != nil { - for i, id := range path { - cmd = c.Sub(id) - - if cmd == nil { - pathS := strings.Join(path[0:i], "/") - res.SetError(fmt.Errorf("Undefined command: '%s'", pathS), Client) - return res - } - - options = append(options, cmd.Options...) - } - } - - optionsMap := make(map[string]Option) - for _, opt := range options { - for _, name := range opt.Names { - optionsMap[name] = opt - } - } - - for k, v := range req.options { - opt, ok := optionsMap[k] - - if !ok { - res.SetError(fmt.Errorf("Unrecognized command option: '%s'", k), Client) - return res - } - - for _, name := range opt.Names { - if _, ok = req.options[name]; name != k && ok { - res.SetError(fmt.Errorf("Duplicate command options were provided ('%s' and '%s')", - k, name), Client) - return res - } - } - - kind := reflect.TypeOf(v).Kind() - if kind != opt.Type { - res.SetError(fmt.Errorf("Option '%s' should be type '%s', but got type '%s'", - k, opt.Type.String(), kind.String()), Client) - return res - } - } - - cmd.f(req, res) - - return res + options := make([]Option, len(c.Options)) + copy(options, c.Options) + options = append(options, globalOptions...) + cmd := c + res := &Response{req: req} + + if path != nil { + for i, id := range path { + cmd = c.Sub(id) + + if cmd == nil { + pathS := strings.Join(path[0:i], "/") + res.SetError(fmt.Errorf("Undefined command: '%s'", pathS), Client) + return res + } + + options = append(options, cmd.Options...) + } + } + + optionsMap := make(map[string]Option) + for _, opt := range options { + for _, name := range opt.Names { + optionsMap[name] = opt + } + } + + for k, v := range req.options { + opt, ok := optionsMap[k] + + if !ok { + res.SetError(fmt.Errorf("Unrecognized command option: '%s'", k), Client) + return res + } + + for _, name := range opt.Names { + if _, ok = req.options[name]; name != k && ok { + res.SetError(fmt.Errorf("Duplicate command options were provided ('%s' and '%s')", + k, name), Client) + return res + } + } + + kind := reflect.TypeOf(v).Kind() + if kind != opt.Type { + res.SetError(fmt.Errorf("Option '%s' should be type '%s', but got type '%s'", + k, opt.Type.String(), kind.String()), Client) + return res + } + } + + cmd.f(req, res) + + return res } // Sub returns the subcommand with the given id func (c *Command) Sub(id string) *Command { - return c.subcommands[id] + return c.subcommands[id] } func (c *Command) checkOptions(names map[string]bool) error { - for _, opt := range c.Options { - for _, name := range opt.Names { - if _, ok := names[name]; ok { - return fmt.Errorf("Multiple options are using the same name ('%s')", name) - } - names[name] = true - } - } - - for _, cmd := range c.subcommands { - err := cmd.checkOptions(names) - if err != nil { - return err - } - } - - return nil + for _, opt := range c.Options { + for _, name := range opt.Names { + if _, ok := names[name]; ok { + return fmt.Errorf("Multiple options are using the same name ('%s')", name) + } + names[name] = true + } + } + + for _, cmd := range c.subcommands { + err := cmd.checkOptions(names) + if err != nil { + return err + } + } + + return nil } diff --git a/commands/command_test.go b/commands/command_test.go index 95667afee32..44586b6a323 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -3,122 +3,122 @@ package commands import "testing" func TestOptionValidation(t *testing.T) { - cmd := Command{ - Options: []Option{ - Option{ []string{ "b", "beep" }, Int }, - Option{ []string{ "B", "boop" }, String }, - }, - f: func(req *Request, res *Response) {}, - } - - req := NewRequest() - req.options["foo"] = 5 - res := cmd.Call(nil, req) - if res.Error == nil { - t.Error("Should have failed (unrecognized command)") - } - - req = NewRequest() - req.options["beep"] = 5 - req.options["b"] = 10 - res = cmd.Call(nil, req) - if res.Error == nil { - t.Error("Should have failed (duplicate options)") - } - - req = NewRequest() - req.options["beep"] = "foo" - res = cmd.Call(nil, req) - if res.Error == nil { - t.Error("Should have failed (incorrect type)") - } - - req = NewRequest() - req.options["beep"] = 5 - res = cmd.Call(nil, req) - if res.Error != nil { - t.Error("Should have passed") - } - - req = NewRequest() - req.options["beep"] = 5 - req.options["boop"] = "test" - res = cmd.Call(nil, req) - if res.Error != nil { - t.Error("Should have passed") - } - - req = NewRequest() - req.options["b"] = 5 - req.options["B"] = "test" - res = cmd.Call(nil, req) - if res.Error != nil { - t.Error("Should have passed") - } - - req = NewRequest() - req.options["enc"] = "json" - res = cmd.Call(nil, req) - if res.Error != nil { - t.Error("Should have passed") - } + cmd := Command{ + Options: []Option{ + Option{[]string{"b", "beep"}, Int}, + Option{[]string{"B", "boop"}, String}, + }, + f: func(req *Request, res *Response) {}, + } + + req := NewRequest() + req.options["foo"] = 5 + res := cmd.Call(nil, req) + if res.Error == nil { + t.Error("Should have failed (unrecognized command)") + } + + req = NewRequest() + req.options["beep"] = 5 + req.options["b"] = 10 + res = cmd.Call(nil, req) + if res.Error == nil { + t.Error("Should have failed (duplicate options)") + } + + req = NewRequest() + req.options["beep"] = "foo" + res = cmd.Call(nil, req) + if res.Error == nil { + t.Error("Should have failed (incorrect type)") + } + + req = NewRequest() + req.options["beep"] = 5 + res = cmd.Call(nil, req) + if res.Error != nil { + t.Error("Should have passed") + } + + req = NewRequest() + req.options["beep"] = 5 + req.options["boop"] = "test" + res = cmd.Call(nil, req) + if res.Error != nil { + t.Error("Should have passed") + } + + req = NewRequest() + req.options["b"] = 5 + req.options["B"] = "test" + res = cmd.Call(nil, req) + if res.Error != nil { + t.Error("Should have passed") + } + + req = NewRequest() + req.options["enc"] = "json" + res = cmd.Call(nil, req) + if res.Error != nil { + t.Error("Should have passed") + } } func TestRegistration(t *testing.T) { - cmds := []*Command{ - &Command{ - Options: []Option{ - Option{ []string{ "beep" }, Int }, - }, - f: func(req *Request, res *Response) {}, - }, - - &Command{ - Options: []Option{ - Option{ []string{ "boop" }, Int }, - }, - f: func(req *Request, res *Response) {}, - }, - - &Command{ - Options: []Option{ - Option{ []string{ "boop" }, String }, - }, - f: func(req *Request, res *Response) {}, - }, - - &Command{ - Options: []Option{ - Option{ []string{ "bop" }, String }, - }, - f: func(req *Request, res *Response) {}, - }, - - &Command{ - Options: []Option{ - Option{ []string{ "enc" }, String }, - }, - f: func(req *Request, res *Response) {}, - }, - } - - err := cmds[0].Register("foo", cmds[1]) - if err != nil { - t.Error("Should have passed") - } - - err = cmds[0].Register("bar", cmds[2]) - if err == nil { - t.Error("Should have failed (option name collision)") - } - - err = cmds[0].Register("foo", cmds[3]) - if err == nil { - t.Error("Should have failed (subcommand name collision)") - } - - err = cmds[0].Register("baz", cmds[4]) - if err == nil { - t.Error("Should have failed (option name collision with global options)") - } + cmds := []*Command{ + &Command{ + Options: []Option{ + Option{[]string{"beep"}, Int}, + }, + f: func(req *Request, res *Response) {}, + }, + + &Command{ + Options: []Option{ + Option{[]string{"boop"}, Int}, + }, + f: func(req *Request, res *Response) {}, + }, + + &Command{ + Options: []Option{ + Option{[]string{"boop"}, String}, + }, + f: func(req *Request, res *Response) {}, + }, + + &Command{ + Options: []Option{ + Option{[]string{"bop"}, String}, + }, + f: func(req *Request, res *Response) {}, + }, + + &Command{ + Options: []Option{ + Option{[]string{"enc"}, String}, + }, + f: func(req *Request, res *Response) {}, + }, + } + + err := cmds[0].Register("foo", cmds[1]) + if err != nil { + t.Error("Should have passed") + } + + err = cmds[0].Register("bar", cmds[2]) + if err == nil { + t.Error("Should have failed (option name collision)") + } + + err = cmds[0].Register("foo", cmds[3]) + if err == nil { + t.Error("Should have failed (subcommand name collision)") + } + + err = cmds[0].Register("baz", cmds[4]) + if err == nil { + t.Error("Should have failed (option name collision with global options)") + } } diff --git a/commands/option.go b/commands/option.go index e14b1bf68ec..9cf23162b5a 100644 --- a/commands/option.go +++ b/commands/option.go @@ -3,29 +3,30 @@ package commands import "reflect" const ( - Invalid = reflect.Invalid - Bool = reflect.Bool - Int = reflect.Int - Uint = reflect.Uint - Float = reflect.Float32 - String = reflect.String + Invalid = reflect.Invalid + Bool = reflect.Bool + Int = reflect.Int + Uint = reflect.Uint + Float = reflect.Float32 + String = reflect.String ) // Option is used to specify a field that will be provided by a consumer type Option struct { - Names []string // a list of unique names to - Type reflect.Kind // value must be this type + Names []string // a list of unique names to + Type reflect.Kind // value must be this type - // TODO: add more features(?): - //Default interface{} // the default value (ignored if `Required` is true) - //Required bool // whether or not the option must be provided + // TODO: add more features(?): + //Default interface{} // the default value (ignored if `Required` is true) + //Required bool // whether or not the option must be provided } // options that are used by this package var globalOptions []Option = []Option{ - Option{ []string{ "enc", "encoding" }, String }, + Option{[]string{"enc", "encoding"}, String}, } + // the above array of Options, wrapped in a Command var globalCommand *Command = &Command{ - Options: globalOptions, + Options: globalOptions, } diff --git a/commands/request.go b/commands/request.go index 7373a1a243a..0d47c006bb0 100644 --- a/commands/request.go +++ b/commands/request.go @@ -2,29 +2,29 @@ package commands // Request represents a call to a command from a consumer type Request struct { - options map[string]interface{} - arguments []string + options map[string]interface{} + arguments []string } func (r *Request) Option(name string) interface{} { - return r.options[name] + return r.options[name] } func (r *Request) SetOption(option Option, value interface{}) { - // saves the option value in the map, indexed by each name - // (so commands can retrieve it using any of the names) - for _, name := range option.Names { - r.options[name] = value - } + // saves the option value in the map, indexed by each name + // (so commands can retrieve it using any of the names) + for _, name := range option.Names { + r.options[name] = value + } } func (r *Request) Arguments() []string { - return r.arguments + return r.arguments } func NewRequest() *Request { - return &Request{ - make(map[string]interface{}), - make([]string, 0), - } + return &Request{ + make(map[string]interface{}), + make([]string, 0), + } } diff --git a/commands/response.go b/commands/response.go index 07e91d12ff2..6edaf1a59c2 100644 --- a/commands/response.go +++ b/commands/response.go @@ -1,74 +1,77 @@ package commands import ( - "fmt" - "strings" - "encoding/json" - "encoding/xml" + "encoding/json" + "encoding/xml" + "fmt" + "strings" ) type ErrorType uint + const ( - Normal ErrorType = iota // general errors - Client // error was caused by the client, (e.g. invalid CLI usage) - // TODO: add more types of errors for better error-specific handling + Normal ErrorType = iota // general errors + Client // error was caused by the client, (e.g. invalid CLI usage) + // TODO: add more types of errors for better error-specific handling ) // Error is a struct for marshalling errors type Error struct { - Message string - Code ErrorType + Message string + Code ErrorType } type EncodingType string + const ( - Json = "json" - Xml = "xml" - // TODO: support more encoding types + Json = "json" + Xml = "xml" + // TODO: support more encoding types ) -type Marshaller func(v interface{})([]byte, error) +type Marshaller func(v interface{}) ([]byte, error) + var marshallers = map[EncodingType]Marshaller{ - Json: json.Marshal, - Xml: xml.Marshal, + Json: json.Marshal, + Xml: xml.Marshal, } type Response struct { - req *Request - Error error - ErrorType ErrorType - Value interface{} + req *Request + Error error + ErrorType ErrorType + Value interface{} } func (r *Response) SetError(err error, errType ErrorType) { - r.Error = err - r.ErrorType = errType + r.Error = err + r.ErrorType = errType } func (r *Response) FormatError() Error { - return Error{ r.Error.Error(), r.ErrorType } + return Error{r.Error.Error(), r.ErrorType} } func (r *Response) Marshal() ([]byte, error) { - if r.Error == nil && r.Value == nil { - return nil, fmt.Errorf("No error or value set, there is nothing to marshal") - } + if r.Error == nil && r.Value == nil { + return nil, fmt.Errorf("No error or value set, there is nothing to marshal") + } - enc := r.req.Option("enc") - if enc == nil { - return nil, fmt.Errorf("No encoding type was specified") - } - encType := EncodingType(strings.ToLower(enc.(string))) + enc := r.req.Option("enc") + if enc == nil { + return nil, fmt.Errorf("No encoding type was specified") + } + encType := EncodingType(strings.ToLower(enc.(string))) - marshaller, ok := marshallers[encType] - if !ok { - return nil, fmt.Errorf("No marshaller found for encoding type '%s'", enc) - } + marshaller, ok := marshallers[encType] + if !ok { + return nil, fmt.Errorf("No marshaller found for encoding type '%s'", enc) + } - if r.Error != nil { - err := r.FormatError() - return marshaller(err) - } else { - return marshaller(r.Value) - } + if r.Error != nil { + err := r.FormatError() + return marshaller(err) + } else { + return marshaller(r.Value) + } } diff --git a/commands/response_test.go b/commands/response_test.go index e1a96676d23..35f8e488f5e 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -1,45 +1,45 @@ package commands import ( - "testing" - "fmt" + "fmt" + "testing" ) type TestOutput struct { - Foo, Bar string - Baz int + Foo, Bar string + Baz int } func TestMarshalling(t *testing.T) { - req := NewRequest() + req := NewRequest() - res := Response{ - req: req, - Value: TestOutput{ "beep", "boop", 1337 }, - } + res := Response{ + req: req, + Value: TestOutput{"beep", "boop", 1337}, + } - _, err := res.Marshal() - if err == nil { - t.Error("Should have failed (no encoding type specified in request)") - } + _, err := res.Marshal() + if err == nil { + t.Error("Should have failed (no encoding type specified in request)") + } - req.SetOption(globalOptions[0], Json) - bytes, err := res.Marshal() - if err != nil { - t.Error("Should have passed") - } - output := string(bytes) - if output != "{\"Foo\":\"beep\",\"Bar\":\"boop\",\"Baz\":1337}" { - t.Error("Incorrect JSON output") - } + req.SetOption(globalOptions[0], Json) + bytes, err := res.Marshal() + if err != nil { + t.Error("Should have passed") + } + output := string(bytes) + if output != "{\"Foo\":\"beep\",\"Bar\":\"boop\",\"Baz\":1337}" { + t.Error("Incorrect JSON output") + } - res.SetError(fmt.Errorf("You broke something!"), Client) - bytes, err = res.Marshal() - if err != nil { - t.Error("Should have passed") - } - output = string(bytes) - if output != "{\"Message\":\"You broke something!\",\"Code\":1}" { - t.Error("Incorrect JSON output") - } + res.SetError(fmt.Errorf("You broke something!"), Client) + bytes, err = res.Marshal() + if err != nil { + t.Error("Should have passed") + } + output = string(bytes) + if output != "{\"Message\":\"You broke something!\",\"Code\":1}" { + t.Error("Incorrect JSON output") + } } From bf328181cd7d8dacc2b15f537530956161b4dde3 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 13 Oct 2014 19:10:59 -0700 Subject: [PATCH 22/73] commands/cli: Added CLI option parsing --- commands/cli/parse.go | 92 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 commands/cli/parse.go diff --git a/commands/cli/parse.go b/commands/cli/parse.go new file mode 100644 index 00000000000..84e0c2a2c03 --- /dev/null +++ b/commands/cli/parse.go @@ -0,0 +1,92 @@ +package cli + +import ( + "strings" + "fmt" + + "github.com/jbenet/go-ipfs/commands" +) + +func Parse(input []string, root *commands.Command) ([]string, []string, map[string]string, error) { + opts, input, err := options(input, root) + if err != nil { + return nil, nil, nil, err + } + + return nil, nil, opts, nil +} + +// options parses the raw string values of the given options +// returns the parsed options as strings, along with the CLI input minus option blobs +func options(input []string, root *commands.Command) (map[string]string, []string, error) { + opts := make(map[string]string) + cleanInput := make([]string, 0) + + // TODO: error if one option is defined multiple times + + for i := 0; i < len(input); i++ { + blob := input[i] + + if strings.HasPrefix(blob, "--") { + name := blob[2:] + value := "" + + if strings.Contains(name, "=") { + split := strings.SplitN(name, "=", 2) + name = split[0] + value = split[1] + } + + if strings.Contains(name, "-") { + return nil, nil, fmt.Errorf("Invalid option blob: '%s' (Shouldn't contain '-')", input[i]) + } + + if value != "" && strings.Contains(value, "\"") { + // TODO: ignore escaped quotations (--foo="\"") + if !strings.HasPrefix(value, "\"") { + return nil, nil, fmt.Errorf("Invalid option blob: '%s' (Quotation wasn't at the start of value)", input[i]) + } + + value = value[1:] + + for { + if strings.HasSuffix(value, "\"") { + value = value[:len(value)-1] + break + } + + i++ + if i >= len(input) { + return nil, nil, fmt.Errorf("Unterminated string: '%s'", value) + } + + value += " " + input[i] + } + + if strings.Contains(value, "\"") { + return nil, nil, fmt.Errorf("Invalid option blob: '%s' (Value contains unescaped quotation)", value) + } + } + + opts[name] = value + + } else if strings.HasPrefix(blob, "-") { + blob = blob[1:] + + if strings.ContainsAny(blob, "-=\"") { + return nil, nil, fmt.Errorf("Invalid option blob: '%s'", input[i]) + } + + for _, name := range blob { + opts[string(name)] = "" + } + + // TODO: interpret next blob as value if the last option isn't a bool + + } else { + cleanInput = append(cleanInput, blob) + } + } + + return opts, cleanInput, nil +} From b3eecf4f64aca4be377d1a1ded0c48670308d28c Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 13 Oct 2014 19:18:49 -0700 Subject: [PATCH 23/73] commands/cli: Added simple option parser test --- commands/cli/parse_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 commands/cli/parse_test.go diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go new file mode 100644 index 00000000000..6082f95941f --- /dev/null +++ b/commands/cli/parse_test.go @@ -0,0 +1,23 @@ +package cli + +import ( + //"fmt" + "testing" +) + +func TestOptionParsing(t *testing.T) { + opts, input, err := options([]string{ "test", "--beep", "--boop=\"5", "lol\"", "test2", "-cV" }, nil) + /*for k, v := range opts { + fmt.Printf("%s: %s\n", k, v) + } + fmt.Printf("%s\n", input)*/ + if err != nil { + t.Error("Should have passed") + } + if len(opts) != 4 || opts["c"] != "" || opts["V"] != "" || opts["beep"] != "" || opts["boop"] != "5 lol" { + t.Error("Returned options were defferent than expected: %v", opts) + } + if len(input) != 2 || input[0] != "test" || input[1] != "test2" { + t.Error("Returned input was different than expected: %v", input) + } +} From 4bd3a777786a5bd486fd6b481079847d5a4df3dd Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 12:48:26 -0700 Subject: [PATCH 24/73] commands/cli: Added path/args parsing --- commands/cli/parse.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 84e0c2a2c03..5997fb12d19 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -13,7 +13,12 @@ func Parse(input []string, root *commands.Command) ([]string, []string, map[stri return nil, nil, nil, err } - return nil, nil, opts, nil + path, args, err := path(input, root) + if err != nil { + return nil, nil, nil, err + } + + return path, args, opts, nil } // options parses the raw string values of the given options @@ -90,3 +95,21 @@ func options(input []string, root *commands.Command) (map[string]string, []strin return opts, cleanInput, nil } + +// path takes the command line (without options) and splits it into the command path and arguments +func path(input []string, root *commands.Command) ([]string, []string, error) { + cmd := root + i := 0 + + for _, blob := range input { + cmd := cmd.Sub(blob) + + if cmd == nil { + break + } + + i++ + } + + return input[:i], input[i:], nil +} From f437230d8871de33c676f359eddd45d1367416b5 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 12:48:38 -0700 Subject: [PATCH 25/73] commands/cli: Added path/args test --- commands/cli/parse_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 6082f95941f..b1a051ad4c8 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -3,6 +3,8 @@ package cli import ( //"fmt" "testing" + + "github.com/jbenet/go-ipfs/commands" ) func TestOptionParsing(t *testing.T) { @@ -20,4 +22,17 @@ func TestOptionParsing(t *testing.T) { if len(input) != 2 || input[0] != "test" || input[1] != "test2" { t.Error("Returned input was different than expected: %v", input) } + + cmd := &commands.Command{} + cmd.Register("test", &commands.Command{}) + path, args, err := path([]string{ "test", "beep", "boop" }, cmd) + if err != nil { + t.Error("Should have passed") + } + if len(path) != 1 || path[0] != "test" { + t.Error("Returned path was defferent than expected: %v", path) + } + if len(args) != 2 || args[0] != "beep" || args[1] != "boop" { + t.Error("Returned args were different than expected: %v", args) + } } From 1b3561582450084313482b8db6c2b7e4a662ab4d Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 14:27:56 -0700 Subject: [PATCH 26/73] commands: Made Command#GetOption method, for getting all options for a given command path --- commands/command.go | 59 +++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/commands/command.go b/commands/command.go index a98b1989a5c..908f949cd65 100644 --- a/commands/command.go +++ b/commands/command.go @@ -38,35 +38,17 @@ func (c *Command) Register(id string, sub *Command) error { // Call invokes the command at the given subcommand path func (c *Command) Call(path []string, req *Request) *Response { - options := make([]Option, len(c.Options)) - copy(options, c.Options) - options = append(options, globalOptions...) cmd := c res := &Response{req: req} - if path != nil { - for i, id := range path { - cmd = c.Sub(id) - - if cmd == nil { - pathS := strings.Join(path[0:i], "/") - res.SetError(fmt.Errorf("Undefined command: '%s'", pathS), Client) - return res - } - - options = append(options, cmd.Options...) - } - } - - optionsMap := make(map[string]Option) - for _, opt := range options { - for _, name := range opt.Names { - optionsMap[name] = opt - } - } + options, err := cmd.GetOptions(path) + if err != nil { + res.SetError(err, Client) + return res + } for k, v := range req.options { - opt, ok := optionsMap[k] + opt, ok := options[k] if !ok { res.SetError(fmt.Errorf("Unrecognized command option: '%s'", k), Client) @@ -94,6 +76,35 @@ func (c *Command) Call(path []string, req *Request) *Response { return res } +// GetOptions gets the options in the given path of commands +func (c *Command) GetOptions(path []string) (map[string]Option, error) { + options := make([]Option, len(c.Options)) + copy(options, c.Options) + options = append(options, globalOptions...) + + if path != nil { + for i, id := range path { + cmd := c.Sub(id) + + if cmd == nil { + pathS := strings.Join(path[0:i], "/") + return nil, fmt.Errorf("Undefined command: '%s'", pathS) + } + + options = append(options, cmd.Options...) + } + } + + optionsMap := make(map[string]Option) + for _, opt := range options { + for _, name := range opt.Names { + optionsMap[name] = opt + } + } + + return optionsMap, nil +} + // Sub returns the subcommand with the given id func (c *Command) Sub(id string) *Command { return c.subcommands[id] From 66b0727de61ca7ee5ff64fe13e8c3df5c41ef93f Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 14:29:08 -0700 Subject: [PATCH 27/73] commands/cli: Renamed parse functions to parse* --- commands/cli/parse.go | 8 ++++---- commands/cli/parse_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 5997fb12d19..0f297fd2754 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -8,12 +8,12 @@ import ( ) func Parse(input []string, root *commands.Command) ([]string, []string, map[string]string, error) { - opts, input, err := options(input, root) + opts, input, err := parseOptions(input, root) if err != nil { return nil, nil, nil, err } - path, args, err := path(input, root) + path, args, err := parsePath(input, root) if err != nil { return nil, nil, nil, err } @@ -23,7 +23,7 @@ func Parse(input []string, root *commands.Command) ([]string, []string, map[stri // options parses the raw string values of the given options // returns the parsed options as strings, along with the CLI input minus option blobs -func options(input []string, root *commands.Command) (map[string]string, []string, error) { +func parseOptions(input []string, root *commands.Command) (map[string]string, []string, error) { opts := make(map[string]string) cleanInput := make([]string, 0) @@ -97,7 +97,7 @@ func options(input []string, root *commands.Command) (map[string]string, []strin } // path takes the command line (without options) and splits it into the command path and arguments -func path(input []string, root *commands.Command) ([]string, []string, error) { +func parsePath(input []string, root *commands.Command) ([]string, []string, error) { cmd := root i := 0 diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index b1a051ad4c8..e3197b6f1ac 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -8,7 +8,7 @@ import ( ) func TestOptionParsing(t *testing.T) { - opts, input, err := options([]string{ "test", "--beep", "--boop=\"5", "lol\"", "test2", "-cV" }, nil) + opts, input, err := parseOptions([]string{ "test", "--beep", "--boop=\"5", "lol\"", "test2", "-cV" }, nil) /*for k, v := range opts { fmt.Printf("%s: %s\n", k, v) } @@ -25,7 +25,7 @@ func TestOptionParsing(t *testing.T) { cmd := &commands.Command{} cmd.Register("test", &commands.Command{}) - path, args, err := path([]string{ "test", "beep", "boop" }, cmd) + path, args, err := parsePath([]string{ "test", "beep", "boop" }, cmd) if err != nil { t.Error("Should have passed") } From bb32633136813799957eb0a403533ac5342f258b Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 15:00:20 -0700 Subject: [PATCH 28/73] commands/cli: Refactored parsing to always get the command path at the beginning of the CLI input --- commands/cli/parse.go | 59 +++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 0f297fd2754..13391e1f161 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -8,12 +8,12 @@ import ( ) func Parse(input []string, root *commands.Command) ([]string, []string, map[string]string, error) { - opts, input, err := parseOptions(input, root) + path, input, err := parsePath(input, root) if err != nil { return nil, nil, nil, err } - path, args, err := parsePath(input, root) + opts, args, err := parseOptions(input, path, root) if err != nil { return nil, nil, nil, err } @@ -21,11 +21,38 @@ func Parse(input []string, root *commands.Command) ([]string, []string, map[stri return path, args, opts, nil } + +// path gets the command path from the command line input +func parsePath(input []string, root *commands.Command) ([]string, []string, error) { + cmd := root + i := 0 + + for _, blob := range input { + if strings.HasPrefix(blob, "-") { + break + } + + cmd := cmd.Sub(blob) + if cmd == nil { + break + } + + i++ + } + + return input[:i], input[i:], nil +} + // options parses the raw string values of the given options -// returns the parsed options as strings, along with the CLI input minus option blobs -func parseOptions(input []string, root *commands.Command) (map[string]string, []string, error) { +// returns the parsed options as strings, along with the CLI args +func parseOptions(input, path []string, root *commands.Command) (map[string]string, []string, error) { + _, err := root.GetOptions(path) + if err != nil { + return nil, nil, err + } + opts := make(map[string]string) - cleanInput := make([]string, 0) + args := make([]string, 0) // TODO: error if one option is defined multiple times @@ -89,27 +116,9 @@ func parseOptions(input []string, root *commands.Command) (map[string]string, [] // TODO: interpret next blob as value if the last option isn't a bool } else { - cleanInput = append(cleanInput, blob) - } - } - - return opts, cleanInput, nil -} - -// path takes the command line (without options) and splits it into the command path and arguments -func parsePath(input []string, root *commands.Command) ([]string, []string, error) { - cmd := root - i := 0 - - for _, blob := range input { - cmd := cmd.Sub(blob) - - if cmd == nil { - break + args = append(args, blob) } - - i++ } - return input[:i], input[i:], nil + return opts, args, nil } From 08885c0ced2c2974dc500a3d763fab5eede0bb1f Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 15:00:32 -0700 Subject: [PATCH 29/73] commands/cli: Fixed tests for refactor --- commands/cli/parse_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index e3197b6f1ac..ba00b57f993 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -8,7 +8,11 @@ import ( ) func TestOptionParsing(t *testing.T) { - opts, input, err := parseOptions([]string{ "test", "--beep", "--boop=\"5", "lol\"", "test2", "-cV" }, nil) + cmd := &commands.Command{} + cmd.Register("test", &commands.Command{}) + + opts, input, err := parseOptions([]string{ "--beep", "--boop=\"5", "lol\"", "test2", "-cV" }, + []string{"test"}, cmd) /*for k, v := range opts { fmt.Printf("%s: %s\n", k, v) } @@ -19,12 +23,10 @@ func TestOptionParsing(t *testing.T) { if len(opts) != 4 || opts["c"] != "" || opts["V"] != "" || opts["beep"] != "" || opts["boop"] != "5 lol" { t.Error("Returned options were defferent than expected: %v", opts) } - if len(input) != 2 || input[0] != "test" || input[1] != "test2" { + if len(input) != 1 || input[0] != "test2" { t.Error("Returned input was different than expected: %v", input) } - cmd := &commands.Command{} - cmd.Register("test", &commands.Command{}) path, args, err := parsePath([]string{ "test", "beep", "boop" }, cmd) if err != nil { t.Error("Should have passed") From 66e6da3ddd09832575017271bda8164e813bc171 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 15:15:42 -0700 Subject: [PATCH 30/73] commands/cli: Added value parsing for single-dash options --- commands/cli/parse.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 13391e1f161..f766af43640 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -46,7 +46,7 @@ func parsePath(input []string, root *commands.Command) ([]string, []string, erro // options parses the raw string values of the given options // returns the parsed options as strings, along with the CLI args func parseOptions(input, path []string, root *commands.Command) (map[string]string, []string, error) { - _, err := root.GetOptions(path) + options, err := root.GetOptions(path) if err != nil { return nil, nil, err } @@ -109,11 +109,21 @@ func parseOptions(input, path []string, root *commands.Command) (map[string]stri return nil, nil, fmt.Errorf("Invalid option blob: '%s'", input[i]) } + nameS := "" for _, name := range blob { - opts[string(name)] = "" + nameS = string(name) + opts[nameS] = "" } - // TODO: interpret next blob as value if the last option isn't a bool + if nameS != "" { + opt, ok := options[nameS] + if ok && opt.Type != commands.Bool { + i++ + if i <= len(input) { + opts[nameS] = input[i] + } + } + } } else { args = append(args, blob) From 5d9fa93c85b82b5a91b7d4bb77c7bc8001102da7 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 15:16:18 -0700 Subject: [PATCH 31/73] commands/cli: Added test for single-dash option value --- commands/cli/parse_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index ba00b57f993..2ede7b9c812 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -8,10 +8,14 @@ import ( ) func TestOptionParsing(t *testing.T) { - cmd := &commands.Command{} + cmd := &commands.Command{ + Options: []commands.Option{ + commands.Option{ []string{"b"}, commands.String }, + }, + } cmd.Register("test", &commands.Command{}) - opts, input, err := parseOptions([]string{ "--beep", "--boop=\"5", "lol\"", "test2", "-cV" }, + opts, input, err := parseOptions([]string{ "--beep", "--boop=\"5", "lol\"", "test2", "-cVb", "beep" }, []string{"test"}, cmd) /*for k, v := range opts { fmt.Printf("%s: %s\n", k, v) @@ -20,7 +24,7 @@ func TestOptionParsing(t *testing.T) { if err != nil { t.Error("Should have passed") } - if len(opts) != 4 || opts["c"] != "" || opts["V"] != "" || opts["beep"] != "" || opts["boop"] != "5 lol" { + if len(opts) != 5 || opts["c"] != "" || opts["V"] != "" || opts["beep"] != "" || opts["boop"] != "5 lol" || opts["b"] != "beep" { t.Error("Returned options were defferent than expected: %v", opts) } if len(input) != 1 || input[0] != "test2" { From 97b8719075bc5c3f568cca8fd8f74af3d0939197 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 15:37:05 -0700 Subject: [PATCH 32/73] commands/cli: Removed parser string handling since the go runtime handles it for us automatically --- commands/cli/parse.go | 36 ++---------------------------------- commands/cli/parse_test.go | 2 +- 2 files changed, 3 insertions(+), 35 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index f766af43640..bd37419155e 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -21,8 +21,7 @@ func Parse(input []string, root *commands.Command) ([]string, []string, map[stri return path, args, opts, nil } - -// path gets the command path from the command line input +// parsePath gets the command path from the command line input func parsePath(input []string, root *commands.Command) ([]string, []string, error) { cmd := root i := 0 @@ -43,7 +42,7 @@ func parsePath(input []string, root *commands.Command) ([]string, []string, erro return input[:i], input[i:], nil } -// options parses the raw string values of the given options +// parseOptions parses the raw string values of the given options // returns the parsed options as strings, along with the CLI args func parseOptions(input, path []string, root *commands.Command) (map[string]string, []string, error) { options, err := root.GetOptions(path) @@ -69,37 +68,6 @@ func parseOptions(input, path []string, root *commands.Command) (map[string]stri value = split[1] } - if strings.Contains(name, "-") { - return nil, nil, fmt.Errorf("Invalid option blob: '%s' (Shouldn't contain '-')", input[i]) - } - - if value != "" && strings.Contains(value, "\"") { - // TODO: ignore escaped quotations (--foo="\"") - if !strings.HasPrefix(value, "\"") { - return nil, nil, fmt.Errorf("Invalid option blob: '%s' (Quotation wasn't at the start of value)", input[i]) - } - - value = value[1:] - - for { - if strings.HasSuffix(value, "\"") { - value = value[:len(value)-1] - break - } - - i++ - if i >= len(input) { - return nil, nil, fmt.Errorf("Unterminated string: '%s'", value) - } - - value += " " + input[i] - } - - if strings.Contains(value, "\"") { - return nil, nil, fmt.Errorf("Invalid option blob: '%s' (Value contains unescaped quotation)", value) - } - } - opts[name] = value } else if strings.HasPrefix(blob, "-") { diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 2ede7b9c812..247ca3d1e62 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -15,7 +15,7 @@ func TestOptionParsing(t *testing.T) { } cmd.Register("test", &commands.Command{}) - opts, input, err := parseOptions([]string{ "--beep", "--boop=\"5", "lol\"", "test2", "-cVb", "beep" }, + opts, input, err := parseOptions([]string{ "--beep", "--boop=5 lol", "test2", "-cVb", "beep" }, []string{"test"}, cmd) /*for k, v := range opts { fmt.Printf("%s: %s\n", k, v) From 86bc450b20b190ece2d29d39bc4a499bee38939b Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 15:46:49 -0700 Subject: [PATCH 33/73] commands/cli: Pass option definitions as an argument to parseOptions --- commands/cli/parse.go | 14 +++++++------- commands/cli/parse_test.go | 9 +++++++-- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index bd37419155e..ba57880cb97 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -13,7 +13,12 @@ func Parse(input []string, root *commands.Command) ([]string, []string, map[stri return nil, nil, nil, err } - opts, args, err := parseOptions(input, path, root) + options, err := root.GetOptions(path) + if err != nil { + return nil, nil, nil, err + } + + opts, args, err := parseOptions(input, options) if err != nil { return nil, nil, nil, err } @@ -44,12 +49,7 @@ func parsePath(input []string, root *commands.Command) ([]string, []string, erro // parseOptions parses the raw string values of the given options // returns the parsed options as strings, along with the CLI args -func parseOptions(input, path []string, root *commands.Command) (map[string]string, []string, error) { - options, err := root.GetOptions(path) - if err != nil { - return nil, nil, err - } - +func parseOptions(input []string, options map[string]commands.Option) (map[string]string, []string, error) { opts := make(map[string]string) args := make([]string, 0) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 247ca3d1e62..f9f7f707216 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -15,8 +15,13 @@ func TestOptionParsing(t *testing.T) { } cmd.Register("test", &commands.Command{}) - opts, input, err := parseOptions([]string{ "--beep", "--boop=5 lol", "test2", "-cVb", "beep" }, - []string{"test"}, cmd) + path := []string{"test"} + options, err := cmd.GetOptions(path) + if err != nil { + t.Error(err) + } + + opts, input, err := parseOptions([]string{ "--beep", "--boop=5 lol", "test2", "-cVb", "beep" }, options) /*for k, v := range opts { fmt.Printf("%s: %s\n", k, v) } From 793a8de9c248b91903f28eb9ad5ee6dfe78b1875 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 16:25:13 -0700 Subject: [PATCH 34/73] commands: Refactored to make Request contain command path --- commands/command.go | 5 +++-- commands/command_test.go | 14 +++++++------- commands/request.go | 6 ++++++ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/commands/command.go b/commands/command.go index 908f949cd65..787ea532f32 100644 --- a/commands/command.go +++ b/commands/command.go @@ -37,11 +37,11 @@ func (c *Command) Register(id string, sub *Command) error { } // Call invokes the command at the given subcommand path -func (c *Command) Call(path []string, req *Request) *Response { +func (c *Command) Call(req *Request) *Response { cmd := c res := &Response{req: req} - options, err := cmd.GetOptions(path) + options, err := cmd.GetOptions(req.path) if err != nil { res.SetError(err, Client) return res @@ -82,6 +82,7 @@ func (c *Command) GetOptions(path []string) (map[string]Option, error) { copy(options, c.Options) options = append(options, globalOptions...) + // a nil path means this command, not a subcommand (same as an empty path) if path != nil { for i, id := range path { cmd := c.Sub(id) diff --git a/commands/command_test.go b/commands/command_test.go index 44586b6a323..766dfc7c889 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -13,7 +13,7 @@ func TestOptionValidation(t *testing.T) { req := NewRequest() req.options["foo"] = 5 - res := cmd.Call(nil, req) + res := cmd.Call(req) if res.Error == nil { t.Error("Should have failed (unrecognized command)") } @@ -21,21 +21,21 @@ func TestOptionValidation(t *testing.T) { req = NewRequest() req.options["beep"] = 5 req.options["b"] = 10 - res = cmd.Call(nil, req) + res = cmd.Call(req) if res.Error == nil { t.Error("Should have failed (duplicate options)") } req = NewRequest() req.options["beep"] = "foo" - res = cmd.Call(nil, req) + res = cmd.Call(req) if res.Error == nil { t.Error("Should have failed (incorrect type)") } req = NewRequest() req.options["beep"] = 5 - res = cmd.Call(nil, req) + res = cmd.Call(req) if res.Error != nil { t.Error("Should have passed") } @@ -43,7 +43,7 @@ func TestOptionValidation(t *testing.T) { req = NewRequest() req.options["beep"] = 5 req.options["boop"] = "test" - res = cmd.Call(nil, req) + res = cmd.Call(req) if res.Error != nil { t.Error("Should have passed") } @@ -51,14 +51,14 @@ func TestOptionValidation(t *testing.T) { req = NewRequest() req.options["b"] = 5 req.options["B"] = "test" - res = cmd.Call(nil, req) + res = cmd.Call(req) if res.Error != nil { t.Error("Should have passed") } req = NewRequest() req.options["enc"] = "json" - res = cmd.Call(nil, req) + res = cmd.Call(req) if res.Error != nil { t.Error("Should have passed") } diff --git a/commands/request.go b/commands/request.go index 0d47c006bb0..f94801303eb 100644 --- a/commands/request.go +++ b/commands/request.go @@ -2,10 +2,15 @@ package commands // Request represents a call to a command from a consumer type Request struct { + path []string options map[string]interface{} arguments []string } +func (r *Request) Path() []string { + return r.path +} + func (r *Request) Option(name string) interface{} { return r.options[name] } @@ -24,6 +29,7 @@ func (r *Request) Arguments() []string { func NewRequest() *Request { return &Request{ + make([]string, 0), make(map[string]interface{}), make([]string, 0), } From e1a4b8d6682247992a8ea44c4d6aabf5fbd942a3 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 16:26:57 -0700 Subject: [PATCH 35/73] commands: Added Request#SetPath method --- commands/request.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/commands/request.go b/commands/request.go index f94801303eb..65661aa191f 100644 --- a/commands/request.go +++ b/commands/request.go @@ -11,6 +11,10 @@ func (r *Request) Path() []string { return r.path } +func (r *Request) SetPath(path []string) { + r.path = path +} + func (r *Request) Option(name string) interface{} { return r.options[name] } From c575b5087aca984006b045ebe08274abe2a3f2e9 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 17:36:30 -0700 Subject: [PATCH 36/73] commands: Added option value conversion, and moved option validation logic into Request#convertOptions --- commands/command.go | 29 +++-------------- commands/option.go | 2 +- commands/request.go | 77 +++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 77 insertions(+), 31 deletions(-) diff --git a/commands/command.go b/commands/command.go index 787ea532f32..7a99f6eaf95 100644 --- a/commands/command.go +++ b/commands/command.go @@ -2,7 +2,6 @@ package commands import ( "fmt" - "reflect" "strings" ) @@ -47,29 +46,11 @@ func (c *Command) Call(req *Request) *Response { return res } - for k, v := range req.options { - opt, ok := options[k] - - if !ok { - res.SetError(fmt.Errorf("Unrecognized command option: '%s'", k), Client) - return res - } - - for _, name := range opt.Names { - if _, ok = req.options[name]; name != k && ok { - res.SetError(fmt.Errorf("Duplicate command options were provided ('%s' and '%s')", - k, name), Client) - return res - } - } - - kind := reflect.TypeOf(v).Kind() - if kind != opt.Type { - res.SetError(fmt.Errorf("Option '%s' should be type '%s', but got type '%s'", - k, opt.Type.String(), kind.String()), Client) - return res - } - } + err = req.convertOptions(options) + if err != nil { + res.SetError(err, Client) + return res + } cmd.f(req, res) diff --git a/commands/option.go b/commands/option.go index 9cf23162b5a..f58836ec031 100644 --- a/commands/option.go +++ b/commands/option.go @@ -7,7 +7,7 @@ const ( Bool = reflect.Bool Int = reflect.Int Uint = reflect.Uint - Float = reflect.Float32 + Float = reflect.Float64 String = reflect.String ) diff --git a/commands/request.go b/commands/request.go index 65661aa191f..61850917aa1 100644 --- a/commands/request.go +++ b/commands/request.go @@ -1,5 +1,11 @@ package commands +import ( + "fmt" + "reflect" + "strconv" +) + // Request represents a call to a command from a consumer type Request struct { path []string @@ -19,18 +25,77 @@ func (r *Request) Option(name string) interface{} { return r.options[name] } -func (r *Request) SetOption(option Option, value interface{}) { - // saves the option value in the map, indexed by each name - // (so commands can retrieve it using any of the names) - for _, name := range option.Names { - r.options[name] = value - } +func (r *Request) SetOption(name string, value interface{}) { + r.options[name] = value } func (r *Request) Arguments() []string { return r.arguments } +type converter func(string)(interface{}, error) +var converters map[reflect.Kind]converter = map[reflect.Kind]converter{ + Bool: func(v string)(interface{}, error) { + if v == "" { + return true, nil + } + return strconv.ParseBool(v) + }, + Int: func(v string)(interface{}, error) { + return strconv.ParseInt(v, 0, 32) + }, + Uint: func(v string)(interface{}, error) { + return strconv.ParseInt(v, 0, 32) + }, + Float: func(v string)(interface{}, error) { + return strconv.ParseFloat(v, 64) + }, +} + +func (r *Request) convertOptions(options map[string]Option) error { + converted := make(map[string]interface{}) + + for k, v := range r.options { + opt, ok := options[k] + if !ok { + return fmt.Errorf("Unrecognized option: '%s'", k) + } + + kind := reflect.TypeOf(v).Kind() + var value interface{} + + if kind != opt.Type { + if kind == String { + convert := converters[opt.Type] + val, err := convert(v.(string)) + if err != nil { + return fmt.Errorf("Could not convert string value '%s' to type '%s'", + v, opt.Type.String()) + } + value = val + + } else { + return fmt.Errorf("Option '%s' should be type '%s', but got type '%s'", + k, opt.Type.String(), kind.String()) + } + } else { + value = v + } + + for _, name := range opt.Names { + if _, ok := r.options[name]; name != k && ok { + return fmt.Errorf("Duplicate command options were provided ('%s' and '%s')", + k, name) + } + + converted[name] = value + } + } + + r.options = converted + return nil +} + func NewRequest() *Request { return &Request{ make([]string, 0), From 1e8719e7f4acb0342f62bc3fee478a2136622a37 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 17:37:14 -0700 Subject: [PATCH 37/73] commands: Fixed tests --- commands/command_test.go | 4 ++-- commands/response_test.go | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/commands/command_test.go b/commands/command_test.go index 766dfc7c889..47b151fedcb 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -15,7 +15,7 @@ func TestOptionValidation(t *testing.T) { req.options["foo"] = 5 res := cmd.Call(req) if res.Error == nil { - t.Error("Should have failed (unrecognized command)") + t.Error("Should have failed (unrecognized option)") } req = NewRequest() @@ -37,7 +37,7 @@ func TestOptionValidation(t *testing.T) { req.options["beep"] = 5 res = cmd.Call(req) if res.Error != nil { - t.Error("Should have passed") + t.Error(res.Error, "Should have passed") } req = NewRequest() diff --git a/commands/response_test.go b/commands/response_test.go index 35f8e488f5e..fcd6a398a02 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -18,15 +18,24 @@ func TestMarshalling(t *testing.T) { Value: TestOutput{"beep", "boop", 1337}, } - _, err := res.Marshal() + // get command global options so we can set the encoding option + cmd := Command{} + options, err := cmd.GetOptions(nil) + if err != nil { + t.Error(err) + } + + _, err = res.Marshal() if err == nil { t.Error("Should have failed (no encoding type specified in request)") } - req.SetOption(globalOptions[0], Json) + req.SetOption("enc", Json) + req.convertOptions(options) + bytes, err := res.Marshal() if err != nil { - t.Error("Should have passed") + t.Error(err, "Should have passed") } output := string(bytes) if output != "{\"Foo\":\"beep\",\"Bar\":\"boop\",\"Baz\":1337}" { From 47eea7fd93d2f9fce4150451f83218895ba6e785 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 14 Oct 2014 17:39:55 -0700 Subject: [PATCH 38/73] commands: Added a option validation test for convertible string values --- commands/command_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/commands/command_test.go b/commands/command_test.go index 47b151fedcb..6f8c2674d50 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -62,6 +62,20 @@ func TestOptionValidation(t *testing.T) { if res.Error != nil { t.Error("Should have passed") } + + req = NewRequest() + req.options["b"] = "100" + res = cmd.Call(req) + if res.Error != nil { + t.Error("Should have passed") + } + + req = NewRequest() + req.options["b"] = ":)" + res = cmd.Call(req) + if res.Error == nil { + t.Error(res.Error, "Should have failed (string value not convertible to int)") + } } func TestRegistration(t *testing.T) { From 7a36278dbc24955b715b07eac4f311ade7e54719 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 15 Oct 2014 16:35:59 -0700 Subject: [PATCH 39/73] commands: Allow setting Request fields in NewRequest --- commands/request.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/commands/request.go b/commands/request.go index 61850917aa1..a76e97edd10 100644 --- a/commands/request.go +++ b/commands/request.go @@ -96,10 +96,15 @@ func (r *Request) convertOptions(options map[string]Option) error { return nil } -func NewRequest() *Request { - return &Request{ - make([]string, 0), - make(map[string]interface{}), - make([]string, 0), +func NewRequest(path []string, opts map[string]interface{}, args []string) *Request { + if path == nil { + path = make([]string, 0) } + if opts == nil { + opts = make(map[string]interface{}) + } + if args == nil { + args = make([]string, 0) + } + return &Request{path, opts, args} } From 968ec344e55b9c6fad857e969095c74e4bab463f Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 15 Oct 2014 16:36:46 -0700 Subject: [PATCH 40/73] commands/cli: Made Parse return a Request object instead of separate values --- commands/cli/parse.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index ba57880cb97..1ccc716d0c4 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -7,23 +7,23 @@ import ( "github.com/jbenet/go-ipfs/commands" ) -func Parse(input []string, root *commands.Command) ([]string, []string, map[string]string, error) { +func Parse(input []string, root *commands.Command) (*commands.Request, error) { path, input, err := parsePath(input, root) if err != nil { - return nil, nil, nil, err + return nil, err } options, err := root.GetOptions(path) if err != nil { - return nil, nil, nil, err + return nil, err } opts, args, err := parseOptions(input, options) if err != nil { - return nil, nil, nil, err + return nil, err } - return path, args, opts, nil + return commands.NewRequest(path, opts, args), nil } // parsePath gets the command path from the command line input @@ -49,8 +49,8 @@ func parsePath(input []string, root *commands.Command) ([]string, []string, erro // parseOptions parses the raw string values of the given options // returns the parsed options as strings, along with the CLI args -func parseOptions(input []string, options map[string]commands.Option) (map[string]string, []string, error) { - opts := make(map[string]string) +func parseOptions(input []string, options map[string]commands.Option) (map[string]interface{}, []string, error) { + opts := make(map[string]interface{}) args := make([]string, 0) // TODO: error if one option is defined multiple times From 09311d4babdf806f63ed5eb48eb59163ab17cfb8 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 15 Oct 2014 17:01:12 -0700 Subject: [PATCH 41/73] commands: Added 'NewEmptyRequest' --- commands/request.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/commands/request.go b/commands/request.go index a76e97edd10..97ba6891ac1 100644 --- a/commands/request.go +++ b/commands/request.go @@ -96,6 +96,10 @@ func (r *Request) convertOptions(options map[string]Option) error { return nil } +func NewEmptyRequest() *Request { + return NewRequest(nil, nil, nil) +} + func NewRequest(path []string, opts map[string]interface{}, args []string) *Request { if path == nil { path = make([]string, 0) From 4b0f44e441d774ecd71b43542a06918131b92674 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 15 Oct 2014 17:01:28 -0700 Subject: [PATCH 42/73] commands: Fixed tests --- commands/command_test.go | 18 +++++++++--------- commands/response_test.go | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/commands/command_test.go b/commands/command_test.go index 6f8c2674d50..ebe4556caff 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -11,14 +11,14 @@ func TestOptionValidation(t *testing.T) { f: func(req *Request, res *Response) {}, } - req := NewRequest() + req := NewEmptyRequest() req.options["foo"] = 5 res := cmd.Call(req) if res.Error == nil { t.Error("Should have failed (unrecognized option)") } - req = NewRequest() + req = NewEmptyRequest() req.options["beep"] = 5 req.options["b"] = 10 res = cmd.Call(req) @@ -26,21 +26,21 @@ func TestOptionValidation(t *testing.T) { t.Error("Should have failed (duplicate options)") } - req = NewRequest() + req = NewEmptyRequest() req.options["beep"] = "foo" res = cmd.Call(req) if res.Error == nil { t.Error("Should have failed (incorrect type)") } - req = NewRequest() + req = NewEmptyRequest() req.options["beep"] = 5 res = cmd.Call(req) if res.Error != nil { t.Error(res.Error, "Should have passed") } - req = NewRequest() + req = NewEmptyRequest() req.options["beep"] = 5 req.options["boop"] = "test" res = cmd.Call(req) @@ -48,7 +48,7 @@ func TestOptionValidation(t *testing.T) { t.Error("Should have passed") } - req = NewRequest() + req = NewEmptyRequest() req.options["b"] = 5 req.options["B"] = "test" res = cmd.Call(req) @@ -56,21 +56,21 @@ func TestOptionValidation(t *testing.T) { t.Error("Should have passed") } - req = NewRequest() + req = NewEmptyRequest() req.options["enc"] = "json" res = cmd.Call(req) if res.Error != nil { t.Error("Should have passed") } - req = NewRequest() + req = NewEmptyRequest() req.options["b"] = "100" res = cmd.Call(req) if res.Error != nil { t.Error("Should have passed") } - req = NewRequest() + req = NewEmptyRequest() req.options["b"] = ":)" res = cmd.Call(req) if res.Error == nil { diff --git a/commands/response_test.go b/commands/response_test.go index fcd6a398a02..9e76a2c1db1 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -11,7 +11,7 @@ type TestOutput struct { } func TestMarshalling(t *testing.T) { - req := NewRequest() + req := NewEmptyRequest() res := Response{ req: req, From 4af61ad9944523886cded2ff0e09919448fdc025 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 15 Oct 2014 17:08:57 -0700 Subject: [PATCH 43/73] commands: Added Command#Resolve --- commands/command.go | 56 +++++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/commands/command.go b/commands/command.go index 7a99f6eaf95..5db0eb9e377 100644 --- a/commands/command.go +++ b/commands/command.go @@ -3,6 +3,7 @@ package commands import ( "fmt" "strings" + "errors" ) type Command struct { @@ -12,6 +13,8 @@ type Command struct { subcommands map[string]*Command } +var NotCallableError = errors.New("This command can't be called directly. Try one of its subcommands.") + // Register adds a subcommand func (c *Command) Register(id string, sub *Command) error { if c.subcommands == nil { @@ -37,10 +40,21 @@ func (c *Command) Register(id string, sub *Command) error { // Call invokes the command at the given subcommand path func (c *Command) Call(req *Request) *Response { - cmd := c res := &Response{req: req} - options, err := cmd.GetOptions(req.path) + cmds, err := c.Resolve(req.path) + if err != nil { + res.SetError(err, Client) + return res + } + cmd := cmds[len(cmds)-1] + + if(cmd.f == nil) { + res.SetError(NotCallableError, Client) + return res + } + + options, err := c.GetOptions(req.path) if err != nil { res.SetError(err, Client) return res @@ -57,24 +71,38 @@ func (c *Command) Call(req *Request) *Response { return res } +// Resolve gets the subcommands at the given path +func (c *Command) Resolve(path []string) ([]*Command, error) { + cmds := make([]*Command, len(path) + 1) + cmds[0] = c + + cmd := c + for i, name := range path { + cmd = cmd.Sub(name) + + if cmd == nil { + pathS := strings.Join(path[0:i], "/") + return nil, fmt.Errorf("Undefined command: '%s'", pathS) + } + + cmds[i+1] = cmd + } + + return cmds, nil +} + // GetOptions gets the options in the given path of commands func (c *Command) GetOptions(path []string) (map[string]Option, error) { options := make([]Option, len(c.Options)) copy(options, c.Options) options = append(options, globalOptions...) - // a nil path means this command, not a subcommand (same as an empty path) - if path != nil { - for i, id := range path { - cmd := c.Sub(id) - - if cmd == nil { - pathS := strings.Join(path[0:i], "/") - return nil, fmt.Errorf("Undefined command: '%s'", pathS) - } - - options = append(options, cmd.Options...) - } + cmds, err := c.Resolve(path) + if err != nil { + return nil, err + } + for _, cmd := range cmds { + options = append(options, cmd.Options...) } optionsMap := make(map[string]Option) From c054fb3655776881166672a3feec9035ae4a8d5f Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 15 Oct 2014 17:09:20 -0700 Subject: [PATCH 44/73] commands: Added simple Command#Resolve test --- commands/command_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/commands/command_test.go b/commands/command_test.go index ebe4556caff..138faf14505 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -136,3 +136,24 @@ func TestRegistration(t *testing.T) { t.Error("Should have failed (option name collision with global options)") } } + +func TestResolving(t *testing.T) { + cmd := &Command{} + cmdA := &Command{} + cmdB := &Command{} + cmdB2 := &Command{} + cmdC := &Command{} + + cmd.Register("a", cmdA) + cmdA.Register("B", cmdB2) + cmdA.Register("b", cmdB) + cmdB.Register("c", cmdC) + + cmds, err := cmd.Resolve([]string{"a","b","c"}) + if err != nil { + t.Error(err) + } + if len(cmds) != 4 || cmds[0] != cmd || cmds[1] != cmdA || cmds[2] != cmdB || cmds[3] != cmdC { + t.Error("Returned command path is different than expected", cmds) + } +} \ No newline at end of file From d2176c05eb2c823387c60fd75f4bffd14ee56bf9 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 15 Oct 2014 17:19:16 -0700 Subject: [PATCH 45/73] commands: Added Command#Get --- commands/command.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/commands/command.go b/commands/command.go index 5db0eb9e377..449ce2aa6a7 100644 --- a/commands/command.go +++ b/commands/command.go @@ -91,6 +91,14 @@ func (c *Command) Resolve(path []string) ([]*Command, error) { return cmds, nil } +func (c *Command) Get(path []string) (*Command, error) { + cmds, err := c.Resolve(path) + if err != nil { + return nil, err + } + return cmds[len(cmds) - 1], nil +} + // GetOptions gets the options in the given path of commands func (c *Command) GetOptions(path []string) (map[string]Option, error) { options := make([]Option, len(c.Options)) From 4f06c6fdbaeaf4b7d60a7e02fcb6f94d9f1b5270 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 15 Oct 2014 20:51:17 -0700 Subject: [PATCH 46/73] commands: Formatted code --- commands/command.go | 114 +++++++++++++++++++-------------------- commands/command_test.go | 4 +- commands/request.go | 17 +++--- 3 files changed, 68 insertions(+), 67 deletions(-) diff --git a/commands/command.go b/commands/command.go index 449ce2aa6a7..6f880400c0c 100644 --- a/commands/command.go +++ b/commands/command.go @@ -1,9 +1,9 @@ package commands import ( + "errors" "fmt" "strings" - "errors" ) type Command struct { @@ -42,29 +42,29 @@ func (c *Command) Register(id string, sub *Command) error { func (c *Command) Call(req *Request) *Response { res := &Response{req: req} - cmds, err := c.Resolve(req.path) - if err != nil { - res.SetError(err, Client) - return res - } - cmd := cmds[len(cmds)-1] + cmds, err := c.Resolve(req.path) + if err != nil { + res.SetError(err, Client) + return res + } + cmd := cmds[len(cmds)-1] - if(cmd.f == nil) { - res.SetError(NotCallableError, Client) - return res - } + if cmd.f == nil { + res.SetError(NotCallableError, Client) + return res + } - options, err := c.GetOptions(req.path) - if err != nil { - res.SetError(err, Client) - return res - } + options, err := c.GetOptions(req.path) + if err != nil { + res.SetError(err, Client) + return res + } err = req.convertOptions(options) - if err != nil { - res.SetError(err, Client) - return res - } + if err != nil { + res.SetError(err, Client) + return res + } cmd.f(req, res) @@ -73,54 +73,54 @@ func (c *Command) Call(req *Request) *Response { // Resolve gets the subcommands at the given path func (c *Command) Resolve(path []string) ([]*Command, error) { - cmds := make([]*Command, len(path) + 1) - cmds[0] = c + cmds := make([]*Command, len(path)+1) + cmds[0] = c - cmd := c - for i, name := range path { - cmd = cmd.Sub(name) + cmd := c + for i, name := range path { + cmd = cmd.Sub(name) - if cmd == nil { - pathS := strings.Join(path[0:i], "/") - return nil, fmt.Errorf("Undefined command: '%s'", pathS) - } + if cmd == nil { + pathS := strings.Join(path[0:i], "/") + return nil, fmt.Errorf("Undefined command: '%s'", pathS) + } - cmds[i+1] = cmd - } + cmds[i+1] = cmd + } - return cmds, nil + return cmds, nil } func (c *Command) Get(path []string) (*Command, error) { - cmds, err := c.Resolve(path) - if err != nil { - return nil, err - } - return cmds[len(cmds) - 1], nil + cmds, err := c.Resolve(path) + if err != nil { + return nil, err + } + return cmds[len(cmds)-1], nil } // GetOptions gets the options in the given path of commands func (c *Command) GetOptions(path []string) (map[string]Option, error) { - options := make([]Option, len(c.Options)) - copy(options, c.Options) - options = append(options, globalOptions...) - - cmds, err := c.Resolve(path) - if err != nil { - return nil, err - } - for _, cmd := range cmds { - options = append(options, cmd.Options...) - } - - optionsMap := make(map[string]Option) - for _, opt := range options { - for _, name := range opt.Names { - optionsMap[name] = opt - } - } - - return optionsMap, nil + options := make([]Option, len(c.Options)) + copy(options, c.Options) + options = append(options, globalOptions...) + + cmds, err := c.Resolve(path) + if err != nil { + return nil, err + } + for _, cmd := range cmds { + options = append(options, cmd.Options...) + } + + optionsMap := make(map[string]Option) + for _, opt := range options { + for _, name := range opt.Names { + optionsMap[name] = opt + } + } + + return optionsMap, nil } // Sub returns the subcommand with the given id diff --git a/commands/command_test.go b/commands/command_test.go index 138faf14505..f93bdb71ff7 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -149,11 +149,11 @@ func TestResolving(t *testing.T) { cmdA.Register("b", cmdB) cmdB.Register("c", cmdC) - cmds, err := cmd.Resolve([]string{"a","b","c"}) + cmds, err := cmd.Resolve([]string{"a", "b", "c"}) if err != nil { t.Error(err) } if len(cmds) != 4 || cmds[0] != cmd || cmds[1] != cmdA || cmds[2] != cmdB || cmds[3] != cmdC { t.Error("Returned command path is different than expected", cmds) } -} \ No newline at end of file +} diff --git a/commands/request.go b/commands/request.go index 97ba6891ac1..42e9ec0e39f 100644 --- a/commands/request.go +++ b/commands/request.go @@ -8,7 +8,7 @@ import ( // Request represents a call to a command from a consumer type Request struct { - path []string + path []string options map[string]interface{} arguments []string } @@ -33,21 +33,22 @@ func (r *Request) Arguments() []string { return r.arguments } -type converter func(string)(interface{}, error) +type converter func(string) (interface{}, error) + var converters map[reflect.Kind]converter = map[reflect.Kind]converter{ - Bool: func(v string)(interface{}, error) { + Bool: func(v string) (interface{}, error) { if v == "" { return true, nil } return strconv.ParseBool(v) }, - Int: func(v string)(interface{}, error) { + Int: func(v string) (interface{}, error) { return strconv.ParseInt(v, 0, 32) }, - Uint: func(v string)(interface{}, error) { + Uint: func(v string) (interface{}, error) { return strconv.ParseInt(v, 0, 32) }, - Float: func(v string)(interface{}, error) { + Float: func(v string) (interface{}, error) { return strconv.ParseFloat(v, 64) }, } @@ -88,7 +89,7 @@ func (r *Request) convertOptions(options map[string]Option) error { k, name) } - converted[name] = value + converted[name] = value } } @@ -102,7 +103,7 @@ func NewEmptyRequest() *Request { func NewRequest(path []string, opts map[string]interface{}, args []string) *Request { if path == nil { - path = make([]string, 0) + path = make([]string, 0) } if opts == nil { opts = make(map[string]interface{}) From e5e121a6b19cccc92e6e31459e14412a3023fac1 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Wed, 15 Oct 2014 21:20:33 -0700 Subject: [PATCH 47/73] commands: Made Request#Option also return an existence bool --- commands/request.go | 5 +++-- commands/response.go | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/commands/request.go b/commands/request.go index 42e9ec0e39f..b8025a6b3cd 100644 --- a/commands/request.go +++ b/commands/request.go @@ -21,8 +21,9 @@ func (r *Request) SetPath(path []string) { r.path = path } -func (r *Request) Option(name string) interface{} { - return r.options[name] +func (r *Request) Option(name string) (interface{}, bool) { + val, ok := r.options[name] + return val, ok } func (r *Request) SetOption(name string, value interface{}) { diff --git a/commands/response.go b/commands/response.go index 6edaf1a59c2..0791a31a206 100644 --- a/commands/response.go +++ b/commands/response.go @@ -57,8 +57,8 @@ func (r *Response) Marshal() ([]byte, error) { return nil, fmt.Errorf("No error or value set, there is nothing to marshal") } - enc := r.req.Option("enc") - if enc == nil { + enc, ok := r.req.Option("enc") + if !ok || enc.(string) == "" { return nil, fmt.Errorf("No encoding type was specified") } encType := EncodingType(strings.ToLower(enc.(string))) From f87c418eeec296d89e66577d137b2d2d3132b260 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 17 Oct 2014 18:18:59 -0700 Subject: [PATCH 48/73] commands/cli: Refactored CLI parsing to match go tooling conventions --- commands/cli/parse.go | 42 +++++++++--------------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 1ccc716d0c4..10c4bbfc3ba 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -2,7 +2,6 @@ package cli import ( "strings" - "fmt" "github.com/jbenet/go-ipfs/commands" ) @@ -13,12 +12,7 @@ func Parse(input []string, root *commands.Command) (*commands.Request, error) { return nil, err } - options, err := root.GetOptions(path) - if err != nil { - return nil, err - } - - opts, args, err := parseOptions(input, options) + opts, args, err := parseOptions(input) if err != nil { return nil, err } @@ -49,7 +43,7 @@ func parsePath(input []string, root *commands.Command) ([]string, []string, erro // parseOptions parses the raw string values of the given options // returns the parsed options as strings, along with the CLI args -func parseOptions(input []string, options map[string]commands.Option) (map[string]interface{}, []string, error) { +func parseOptions(input []string) (map[string]interface{}, []string, error) { opts := make(map[string]interface{}) args := make([]string, 0) @@ -58,10 +52,15 @@ func parseOptions(input []string, options map[string]commands.Option) (map[strin for i := 0; i < len(input); i++ { blob := input[i] - if strings.HasPrefix(blob, "--") { - name := blob[2:] + if strings.HasPrefix(blob, "-") { + name := blob[1:] value := "" + // support single and double dash + if strings.HasPrefix(name, "-") { + name = name[1:] + } + if strings.Contains(name, "=") { split := strings.SplitN(name, "=", 2) name = split[0] @@ -70,29 +69,6 @@ func parseOptions(input []string, options map[string]commands.Option) (map[strin opts[name] = value - } else if strings.HasPrefix(blob, "-") { - blob = blob[1:] - - if strings.ContainsAny(blob, "-=\"") { - return nil, nil, fmt.Errorf("Invalid option blob: '%s'", input[i]) - } - - nameS := "" - for _, name := range blob { - nameS = string(name) - opts[nameS] = "" - } - - if nameS != "" { - opt, ok := options[nameS] - if ok && opt.Type != commands.Bool { - i++ - if i <= len(input) { - opts[nameS] = input[i] - } - } - } - } else { args = append(args, blob) } From b48b12e4252ee729c95a9d1d87851d03858befd0 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 17 Oct 2014 18:19:22 -0700 Subject: [PATCH 49/73] commands/cli: Fixed test for new parsing --- commands/cli/parse_test.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index f9f7f707216..d9353544e40 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -15,13 +15,7 @@ func TestOptionParsing(t *testing.T) { } cmd.Register("test", &commands.Command{}) - path := []string{"test"} - options, err := cmd.GetOptions(path) - if err != nil { - t.Error(err) - } - - opts, input, err := parseOptions([]string{ "--beep", "--boop=5 lol", "test2", "-cVb", "beep" }, options) + opts, input, err := parseOptions([]string{ "--beep", "-boop=lol", "test2", "-c", "beep", "--foo=5" }) /*for k, v := range opts { fmt.Printf("%s: %s\n", k, v) } @@ -29,10 +23,10 @@ func TestOptionParsing(t *testing.T) { if err != nil { t.Error("Should have passed") } - if len(opts) != 5 || opts["c"] != "" || opts["V"] != "" || opts["beep"] != "" || opts["boop"] != "5 lol" || opts["b"] != "beep" { + if len(opts) != 4 || opts["beep"] != "" || opts["boop"] != "lol" || opts["c"] != "" || opts["foo"] != "5" { t.Error("Returned options were defferent than expected: %v", opts) } - if len(input) != 1 || input[0] != "test2" { + if len(input) != 2 || input[0] != "test2" || input[1] != "beep" { t.Error("Returned input was different than expected: %v", input) } From 117af86ca73d624be7d389ffec5ef606c16fca4d Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 17 Oct 2014 18:23:11 -0700 Subject: [PATCH 50/73] commands/cli: Error if there are duplicate values for an option --- commands/cli/parse.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 10c4bbfc3ba..8651aa658f9 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "strings" "github.com/jbenet/go-ipfs/commands" @@ -47,8 +48,6 @@ func parseOptions(input []string) (map[string]interface{}, []string, error) { opts := make(map[string]interface{}) args := make([]string, 0) - // TODO: error if one option is defined multiple times - for i := 0; i < len(input); i++ { blob := input[i] @@ -67,6 +66,10 @@ func parseOptions(input []string) (map[string]interface{}, []string, error) { value = split[1] } + if _, ok := opts[name]; ok { + return nil, nil, fmt.Errorf("Duplicate values for option '%s'", name) + } + opts[name] = value } else { From a9fa767b09daf2d2c683870090c3a810e7c1b9fe Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Fri, 17 Oct 2014 18:23:31 -0700 Subject: [PATCH 51/73] commands/cli: Added test for detecting duplicate options --- commands/cli/parse_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index d9353544e40..f42cab63748 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -30,6 +30,11 @@ func TestOptionParsing(t *testing.T) { t.Error("Returned input was different than expected: %v", input) } + _, _, err = parseOptions([]string{ "-beep=1", "-boop=2", "-beep=3" }) + if err == nil { + t.Error("Should have failed (duplicate option name)") + } + path, args, err := parsePath([]string{ "test", "beep", "boop" }, cmd) if err != nil { t.Error("Should have passed") From 7673ce6f65ce84179f4a65f40c6516b335f6eefd Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Mon, 20 Oct 2014 07:31:19 -0700 Subject: [PATCH 52/73] fmt, lint, + vet commands/ --- commands/cli/parse.go | 102 +++++++++++++++++++------------------ commands/cli/parse_test.go | 74 +++++++++++++-------------- commands/command.go | 13 +++-- commands/option.go | 5 +- commands/response.go | 49 ++++++++++-------- commands/response_test.go | 4 +- 6 files changed, 130 insertions(+), 117 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 8651aa658f9..757f83e60cb 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -1,81 +1,83 @@ package cli import ( - "fmt" - "strings" + "fmt" + "strings" - "github.com/jbenet/go-ipfs/commands" + "github.com/jbenet/go-ipfs/commands" ) +// Parse parses the input commandline string (cmd, flags, and args). +// returns the corresponding command Request object. func Parse(input []string, root *commands.Command) (*commands.Request, error) { - path, input, err := parsePath(input, root) - if err != nil { - return nil, err - } + path, input, err := parsePath(input, root) + if err != nil { + return nil, err + } - opts, args, err := parseOptions(input) - if err != nil { - return nil, err - } + opts, args, err := parseOptions(input) + if err != nil { + return nil, err + } - return commands.NewRequest(path, opts, args), nil + return commands.NewRequest(path, opts, args), nil } // parsePath gets the command path from the command line input func parsePath(input []string, root *commands.Command) ([]string, []string, error) { - cmd := root - i := 0 + cmd := root + i := 0 - for _, blob := range input { - if strings.HasPrefix(blob, "-") { - break - } + for _, blob := range input { + if strings.HasPrefix(blob, "-") { + break + } - cmd := cmd.Sub(blob) - if cmd == nil { - break - } + cmd := cmd.Sub(blob) + if cmd == nil { + break + } - i++ - } + i++ + } - return input[:i], input[i:], nil + return input[:i], input[i:], nil } // parseOptions parses the raw string values of the given options // returns the parsed options as strings, along with the CLI args func parseOptions(input []string) (map[string]interface{}, []string, error) { - opts := make(map[string]interface{}) - args := make([]string, 0) + opts := make(map[string]interface{}) + args := []string{} - for i := 0; i < len(input); i++ { - blob := input[i] + for i := 0; i < len(input); i++ { + blob := input[i] - if strings.HasPrefix(blob, "-") { - name := blob[1:] - value := "" + if strings.HasPrefix(blob, "-") { + name := blob[1:] + value := "" - // support single and double dash - if strings.HasPrefix(name, "-") { - name = name[1:] - } + // support single and double dash + if strings.HasPrefix(name, "-") { + name = name[1:] + } - if strings.Contains(name, "=") { - split := strings.SplitN(name, "=", 2) - name = split[0] - value = split[1] - } + if strings.Contains(name, "=") { + split := strings.SplitN(name, "=", 2) + name = split[0] + value = split[1] + } - if _, ok := opts[name]; ok { - return nil, nil, fmt.Errorf("Duplicate values for option '%s'", name) - } + if _, ok := opts[name]; ok { + return nil, nil, fmt.Errorf("Duplicate values for option '%s'", name) + } - opts[name] = value + opts[name] = value - } else { - args = append(args, blob) - } - } + } else { + args = append(args, blob) + } + } - return opts, args, nil + return opts, args, nil } diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index f42cab63748..caa82c1d0a4 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -1,48 +1,48 @@ package cli import ( - //"fmt" - "testing" + //"fmt" + "testing" - "github.com/jbenet/go-ipfs/commands" + "github.com/jbenet/go-ipfs/commands" ) func TestOptionParsing(t *testing.T) { - cmd := &commands.Command{ - Options: []commands.Option{ - commands.Option{ []string{"b"}, commands.String }, - }, - } - cmd.Register("test", &commands.Command{}) + cmd := &commands.Command{ + Options: []commands.Option{ + commands.Option{Names: []string{"b"}, Type: commands.String}, + }, + } + cmd.Register("test", &commands.Command{}) - opts, input, err := parseOptions([]string{ "--beep", "-boop=lol", "test2", "-c", "beep", "--foo=5" }) - /*for k, v := range opts { - fmt.Printf("%s: %s\n", k, v) - } - fmt.Printf("%s\n", input)*/ - if err != nil { - t.Error("Should have passed") - } - if len(opts) != 4 || opts["beep"] != "" || opts["boop"] != "lol" || opts["c"] != "" || opts["foo"] != "5" { - t.Error("Returned options were defferent than expected: %v", opts) - } - if len(input) != 2 || input[0] != "test2" || input[1] != "beep" { - t.Error("Returned input was different than expected: %v", input) - } + opts, input, err := parseOptions([]string{"--beep", "-boop=lol", "test2", "-c", "beep", "--foo=5"}) + /*for k, v := range opts { + fmt.Printf("%s: %s\n", k, v) + } + fmt.Printf("%s\n", input)*/ + if err != nil { + t.Error("Should have passed") + } + if len(opts) != 4 || opts["beep"] != "" || opts["boop"] != "lol" || opts["c"] != "" || opts["foo"] != "5" { + t.Error("Returned options were defferent than expected: %v", opts) + } + if len(input) != 2 || input[0] != "test2" || input[1] != "beep" { + t.Error("Returned input was different than expected: %v", input) + } - _, _, err = parseOptions([]string{ "-beep=1", "-boop=2", "-beep=3" }) - if err == nil { - t.Error("Should have failed (duplicate option name)") - } + _, _, err = parseOptions([]string{"-beep=1", "-boop=2", "-beep=3"}) + if err == nil { + t.Error("Should have failed (duplicate option name)") + } - path, args, err := parsePath([]string{ "test", "beep", "boop" }, cmd) - if err != nil { - t.Error("Should have passed") - } - if len(path) != 1 || path[0] != "test" { - t.Error("Returned path was defferent than expected: %v", path) - } - if len(args) != 2 || args[0] != "beep" || args[1] != "boop" { - t.Error("Returned args were different than expected: %v", args) - } + path, args, err := parsePath([]string{"test", "beep", "boop"}, cmd) + if err != nil { + t.Error("Should have passed") + } + if len(path) != 1 || path[0] != "test" { + t.Error("Returned path was defferent than expected: %v", path) + } + if len(args) != 2 || args[0] != "beep" || args[1] != "boop" { + t.Error("Returned args were different than expected: %v", args) + } } diff --git a/commands/command.go b/commands/command.go index 6f880400c0c..c6a2955cfe3 100644 --- a/commands/command.go +++ b/commands/command.go @@ -6,6 +6,7 @@ import ( "strings" ) +// Command is an object that defines a command. type Command struct { Help string Options []Option @@ -13,7 +14,8 @@ type Command struct { subcommands map[string]*Command } -var NotCallableError = errors.New("This command can't be called directly. Try one of its subcommands.") +// ErrNotCallable signals a command that cannot be called. +var ErrNotCallable = errors.New("This command can't be called directly. Try one of its subcommands.") // Register adds a subcommand func (c *Command) Register(id string, sub *Command) error { @@ -44,25 +46,25 @@ func (c *Command) Call(req *Request) *Response { cmds, err := c.Resolve(req.path) if err != nil { - res.SetError(err, Client) + res.SetError(err, ErrClient) return res } cmd := cmds[len(cmds)-1] if cmd.f == nil { - res.SetError(NotCallableError, Client) + res.SetError(ErrNotCallable, ErrClient) return res } options, err := c.GetOptions(req.path) if err != nil { - res.SetError(err, Client) + res.SetError(err, ErrClient) return res } err = req.convertOptions(options) if err != nil { - res.SetError(err, Client) + res.SetError(err, ErrClient) return res } @@ -91,6 +93,7 @@ func (c *Command) Resolve(path []string) ([]*Command, error) { return cmds, nil } +// Get resolves and returns the Command addressed by path func (c *Command) Get(path []string) (*Command, error) { cmds, err := c.Resolve(path) if err != nil { diff --git a/commands/option.go b/commands/option.go index f58836ec031..d4faccec13e 100644 --- a/commands/option.go +++ b/commands/option.go @@ -2,6 +2,7 @@ package commands import "reflect" +// Types of Command options const ( Invalid = reflect.Invalid Bool = reflect.Bool @@ -22,11 +23,11 @@ type Option struct { } // options that are used by this package -var globalOptions []Option = []Option{ +var globalOptions = []Option{ Option{[]string{"enc", "encoding"}, String}, } // the above array of Options, wrapped in a Command -var globalCommand *Command = &Command{ +var globalCommand = &Command{ Options: globalOptions, } diff --git a/commands/response.go b/commands/response.go index 0791a31a206..f8847666b14 100644 --- a/commands/response.go +++ b/commands/response.go @@ -7,11 +7,13 @@ import ( "strings" ) +// ErrorType signfies a category of errors type ErrorType uint +// ErrorTypes convey what category of error ocurred const ( - Normal ErrorType = iota // general errors - Client // error was caused by the client, (e.g. invalid CLI usage) + ErrNormal ErrorType = iota // general errors + ErrClient // error was caused by the client, (e.g. invalid CLI usage) // TODO: add more types of errors for better error-specific handling ) @@ -21,37 +23,44 @@ type Error struct { Code ErrorType } +func (e *Error) Error() string { + return fmt.Sprintf("%d error: %s", e.Code, e.Message) +} + +// EncodingType defines a supported encoding type EncodingType string +// Supported EncodingType constants. const ( - Json = "json" - Xml = "xml" + JSON = "json" + XML = "xml" // TODO: support more encoding types ) +// Marshaller is a function used by coding types. +// TODO this should just be a `coding.Codec` type Marshaller func(v interface{}) ([]byte, error) var marshallers = map[EncodingType]Marshaller{ - Json: json.Marshal, - Xml: xml.Marshal, + JSON: json.Marshal, + XML: xml.Marshal, } +// Response is the result of a command request. Handlers write to the response, +// setting Error or Value. Response is returned to the client. type Response struct { - req *Request - Error error - ErrorType ErrorType - Value interface{} -} - -func (r *Response) SetError(err error, errType ErrorType) { - r.Error = err - r.ErrorType = errType + req *Request + Error *Error + Value interface{} } -func (r *Response) FormatError() Error { - return Error{r.Error.Error(), r.ErrorType} +// SetError updates the response Error. +func (r *Response) SetError(err error, code ErrorType) { + r.Error = &Error{Message: err.Error(), Code: code} } +// Marshal marshals out the response into a buffer. It uses the EncodingType +// on the Request to chose a Marshaller (Codec). func (r *Response) Marshal() ([]byte, error) { if r.Error == nil && r.Value == nil { return nil, fmt.Errorf("No error or value set, there is nothing to marshal") @@ -69,9 +78,7 @@ func (r *Response) Marshal() ([]byte, error) { } if r.Error != nil { - err := r.FormatError() - return marshaller(err) - } else { - return marshaller(r.Value) + return marshaller(r.Error) } + return marshaller(r.Value) } diff --git a/commands/response_test.go b/commands/response_test.go index 9e76a2c1db1..5ca31a37f91 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -30,7 +30,7 @@ func TestMarshalling(t *testing.T) { t.Error("Should have failed (no encoding type specified in request)") } - req.SetOption("enc", Json) + req.SetOption("enc", JSON) req.convertOptions(options) bytes, err := res.Marshal() @@ -42,7 +42,7 @@ func TestMarshalling(t *testing.T) { t.Error("Incorrect JSON output") } - res.SetError(fmt.Errorf("You broke something!"), Client) + res.SetError(fmt.Errorf("You broke something!"), ErrClient) bytes, err = res.Marshal() if err != nil { t.Error("Should have passed") From 09d2277f0a2345d04091de363cc1c158a2628a8f Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Mon, 20 Oct 2014 07:35:56 -0700 Subject: [PATCH 53/73] f -> run, Function type. --- commands/command.go | 18 ++++++++++++------ commands/command_test.go | 12 ++++++------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/commands/command.go b/commands/command.go index c6a2955cfe3..b6d2532072a 100644 --- a/commands/command.go +++ b/commands/command.go @@ -6,11 +6,17 @@ import ( "strings" ) -// Command is an object that defines a command. +// Function is the type of function that Commands use. +// It reads from the Request, and writes results to the Response. +type Function func(*Request, *Response) + +// Command is a runnable command, with input arguments and options (flags). +// It can also have subcommands, to group units of work into sets. type Command struct { - Help string - Options []Option - f func(*Request, *Response) + Help string + Options []Option + + run Function subcommands map[string]*Command } @@ -51,7 +57,7 @@ func (c *Command) Call(req *Request) *Response { } cmd := cmds[len(cmds)-1] - if cmd.f == nil { + if cmd.run == nil { res.SetError(ErrNotCallable, ErrClient) return res } @@ -68,7 +74,7 @@ func (c *Command) Call(req *Request) *Response { return res } - cmd.f(req, res) + cmd.run(req, res) return res } diff --git a/commands/command_test.go b/commands/command_test.go index f93bdb71ff7..8e107bca928 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -8,7 +8,7 @@ func TestOptionValidation(t *testing.T) { Option{[]string{"b", "beep"}, Int}, Option{[]string{"B", "boop"}, String}, }, - f: func(req *Request, res *Response) {}, + run: func(req *Request, res *Response) {}, } req := NewEmptyRequest() @@ -84,35 +84,35 @@ func TestRegistration(t *testing.T) { Options: []Option{ Option{[]string{"beep"}, Int}, }, - f: func(req *Request, res *Response) {}, + run: func(req *Request, res *Response) {}, }, &Command{ Options: []Option{ Option{[]string{"boop"}, Int}, }, - f: func(req *Request, res *Response) {}, + run: func(req *Request, res *Response) {}, }, &Command{ Options: []Option{ Option{[]string{"boop"}, String}, }, - f: func(req *Request, res *Response) {}, + run: func(req *Request, res *Response) {}, }, &Command{ Options: []Option{ Option{[]string{"bop"}, String}, }, - f: func(req *Request, res *Response) {}, + run: func(req *Request, res *Response) {}, }, &Command{ Options: []Option{ Option{[]string{"enc"}, String}, }, - f: func(req *Request, res *Response) {}, + run: func(req *Request, res *Response) {}, }, } From 84fa7bc46dd3821f1cee9166bfd4091748c2fdff Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Mon, 20 Oct 2014 07:52:02 -0700 Subject: [PATCH 54/73] AddOptionNames func --- commands/command.go | 45 ++++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/commands/command.go b/commands/command.go index b6d2532072a..fb34845da4d 100644 --- a/commands/command.go +++ b/commands/command.go @@ -30,15 +30,11 @@ func (c *Command) Register(id string, sub *Command) error { } // check for duplicate option names (only checks downwards) - names := make(map[string]bool) - globalCommand.checkOptions(names) - c.checkOptions(names) - err := sub.checkOptions(names) - if err != nil { + if err := checkOptionClashes(globalCommand, c, sub); err != nil { return err } - if _, ok := c.subcommands[id]; ok { + if _, found := c.subcommands[id]; found { return fmt.Errorf("There is already a subcommand registered with id '%s'", id) } @@ -137,19 +133,42 @@ func (c *Command) Sub(id string) *Command { return c.subcommands[id] } -func (c *Command) checkOptions(names map[string]bool) error { +// AddOptionNames returns a map of all command options names, and the command +// they belong to. Will error if names clash in the command hierarchy. +func AddOptionNames(c *Command, names map[string]*Command) error { + for _, opt := range c.Options { for _, name := range opt.Names { - if _, ok := names[name]; ok { - return fmt.Errorf("Multiple options are using the same name ('%s')", name) + if c2, found := names[name]; found { + + // option can be reused in same command, but more often than not + // the clash will be across commands so error out with that, as + // commands tell us where the problem is + errstr := "Option name ('%s') used multiple times (%v, %v)" + return fmt.Errorf(errstr, c2, c) } - names[name] = true + + // mark the name as in use + names[name] = c + } + } + + // for every subcommand, recurse + for _, c2 := range c.subcommands { + if err := AddOptionNames(c2, names); err != nil { + return err } } - for _, cmd := range c.subcommands { - err := cmd.checkOptions(names) - if err != nil { + return nil +} + +// checkOptionClashes checks all command option names for clashes +func checkOptionClashes(cmds ...*Command) error { + names := map[string]*Command{} + + for _, c := range cmds { + if err := AddOptionNames(c, names); err != nil { return err } } From 92528ba7642abcfc6754c7b68b435e65db2e7d31 Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Mon, 20 Oct 2014 07:55:23 -0700 Subject: [PATCH 55/73] Sub -> Subcommand --- commands/cli/parse.go | 2 +- commands/command.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 757f83e60cb..a58298f1744 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -33,7 +33,7 @@ func parsePath(input []string, root *commands.Command) ([]string, []string, erro break } - cmd := cmd.Sub(blob) + cmd := cmd.Subcommand(blob) if cmd == nil { break } diff --git a/commands/command.go b/commands/command.go index fb34845da4d..64b63903bc4 100644 --- a/commands/command.go +++ b/commands/command.go @@ -82,7 +82,7 @@ func (c *Command) Resolve(path []string) ([]*Command, error) { cmd := c for i, name := range path { - cmd = cmd.Sub(name) + cmd = cmd.Subcommand(name) if cmd == nil { pathS := strings.Join(path[0:i], "/") @@ -128,8 +128,8 @@ func (c *Command) GetOptions(path []string) (map[string]Option, error) { return optionsMap, nil } -// Sub returns the subcommand with the given id -func (c *Command) Sub(id string) *Command { +// Subcommand returns the subcommand with the given id +func (c *Command) Subcommand(id string) *Command { return c.subcommands[id] } From 4986600e5455ce98f1cddf79da1915399c47e26a Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Mon, 20 Oct 2014 07:59:19 -0700 Subject: [PATCH 56/73] parsePath no err --- commands/cli/parse.go | 10 +++------- commands/cli/parse_test.go | 5 +---- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index a58298f1744..23a7e1976f8 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -10,11 +10,7 @@ import ( // Parse parses the input commandline string (cmd, flags, and args). // returns the corresponding command Request object. func Parse(input []string, root *commands.Command) (*commands.Request, error) { - path, input, err := parsePath(input, root) - if err != nil { - return nil, err - } - + path, input := parsePath(input, root) opts, args, err := parseOptions(input) if err != nil { return nil, err @@ -24,7 +20,7 @@ func Parse(input []string, root *commands.Command) (*commands.Request, error) { } // parsePath gets the command path from the command line input -func parsePath(input []string, root *commands.Command) ([]string, []string, error) { +func parsePath(input []string, root *commands.Command) ([]string, []string) { cmd := root i := 0 @@ -41,7 +37,7 @@ func parsePath(input []string, root *commands.Command) ([]string, []string, erro i++ } - return input[:i], input[i:], nil + return input[:i], input[i:] } // parseOptions parses the raw string values of the given options diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index caa82c1d0a4..dffc4b1430c 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -35,10 +35,7 @@ func TestOptionParsing(t *testing.T) { t.Error("Should have failed (duplicate option name)") } - path, args, err := parsePath([]string{"test", "beep", "boop"}, cmd) - if err != nil { - t.Error("Should have passed") - } + path, args := parsePath([]string{"test", "beep", "boop"}, cmd) if len(path) != 1 || path[0] != "test" { t.Error("Returned path was defferent than expected: %v", path) } From bbef82f4fa4a6821a614d56c98cbbbe4b424e49f Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Mon, 20 Oct 2014 07:59:39 -0700 Subject: [PATCH 57/73] "enc" -> EncShort --- commands/command_test.go | 4 ++-- commands/option.go | 8 +++++++- commands/response.go | 2 +- commands/response_test.go | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/commands/command_test.go b/commands/command_test.go index 8e107bca928..2f1601f4f23 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -57,7 +57,7 @@ func TestOptionValidation(t *testing.T) { } req = NewEmptyRequest() - req.options["enc"] = "json" + req.options[EncShort] = "json" res = cmd.Call(req) if res.Error != nil { t.Error("Should have passed") @@ -110,7 +110,7 @@ func TestRegistration(t *testing.T) { &Command{ Options: []Option{ - Option{[]string{"enc"}, String}, + Option{[]string{EncShort}, String}, }, run: func(req *Request, res *Response) {}, }, diff --git a/commands/option.go b/commands/option.go index d4faccec13e..8dbf3c1bd54 100644 --- a/commands/option.go +++ b/commands/option.go @@ -22,9 +22,15 @@ type Option struct { //Required bool // whether or not the option must be provided } +// Flag names +const ( + EncShort = "enc" + EncLong = "encoding" +) + // options that are used by this package var globalOptions = []Option{ - Option{[]string{"enc", "encoding"}, String}, + Option{[]string{EncShort, EncLong}, String}, } // the above array of Options, wrapped in a Command diff --git a/commands/response.go b/commands/response.go index f8847666b14..c9d1c5b9302 100644 --- a/commands/response.go +++ b/commands/response.go @@ -66,7 +66,7 @@ func (r *Response) Marshal() ([]byte, error) { return nil, fmt.Errorf("No error or value set, there is nothing to marshal") } - enc, ok := r.req.Option("enc") + enc, ok := r.req.Option(EncShort) if !ok || enc.(string) == "" { return nil, fmt.Errorf("No encoding type was specified") } diff --git a/commands/response_test.go b/commands/response_test.go index 5ca31a37f91..432d78d6978 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -30,7 +30,7 @@ func TestMarshalling(t *testing.T) { t.Error("Should have failed (no encoding type specified in request)") } - req.SetOption("enc", JSON) + req.SetOption(EncShort, JSON) req.convertOptions(options) bytes, err := res.Marshal() From b10fc2cc50f5cc05271b11b10ac41df0883f8d3c Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Mon, 20 Oct 2014 11:49:07 -0700 Subject: [PATCH 58/73] turned req + res into interfaces --- commands/cli/parse.go | 2 +- commands/command.go | 16 ++++++---- commands/command_test.go | 55 +++++++++++++++++---------------- commands/request.go | 49 ++++++++++++++++++----------- commands/response.go | 65 ++++++++++++++++++++++++++++++--------- commands/response_test.go | 8 ++--- 6 files changed, 124 insertions(+), 71 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 23a7e1976f8..a953d0c5e26 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -9,7 +9,7 @@ import ( // Parse parses the input commandline string (cmd, flags, and args). // returns the corresponding command Request object. -func Parse(input []string, root *commands.Command) (*commands.Request, error) { +func Parse(input []string, root *commands.Command) (commands.Request, error) { path, input := parsePath(input, root) opts, args, err := parseOptions(input) if err != nil { diff --git a/commands/command.go b/commands/command.go index 64b63903bc4..cf9472dc2f7 100644 --- a/commands/command.go +++ b/commands/command.go @@ -4,11 +4,15 @@ import ( "errors" "fmt" "strings" + + u "github.com/jbenet/go-ipfs/util" ) +var log = u.Logger("command") + // Function is the type of function that Commands use. // It reads from the Request, and writes results to the Response. -type Function func(*Request, *Response) +type Function func(Request, Response) // Command is a runnable command, with input arguments and options (flags). // It can also have subcommands, to group units of work into sets. @@ -43,10 +47,10 @@ func (c *Command) Register(id string, sub *Command) error { } // Call invokes the command at the given subcommand path -func (c *Command) Call(req *Request) *Response { - res := &Response{req: req} +func (c *Command) Call(req Request) Response { + res := NewResponse(req) - cmds, err := c.Resolve(req.path) + cmds, err := c.Resolve(req.Path()) if err != nil { res.SetError(err, ErrClient) return res @@ -58,13 +62,13 @@ func (c *Command) Call(req *Request) *Response { return res } - options, err := c.GetOptions(req.path) + options, err := c.GetOptions(req.Path()) if err != nil { res.SetError(err, ErrClient) return res } - err = req.convertOptions(options) + err = req.ConvertOptions(options) if err != nil { res.SetError(err, ErrClient) return res diff --git a/commands/command_test.go b/commands/command_test.go index 2f1601f4f23..6850fa40135 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -8,70 +8,70 @@ func TestOptionValidation(t *testing.T) { Option{[]string{"b", "beep"}, Int}, Option{[]string{"B", "boop"}, String}, }, - run: func(req *Request, res *Response) {}, + run: func(req Request, res Response) {}, } req := NewEmptyRequest() - req.options["foo"] = 5 + req.SetOption("foo", 5) res := cmd.Call(req) - if res.Error == nil { + if res.Error() == nil { t.Error("Should have failed (unrecognized option)") } req = NewEmptyRequest() - req.options["beep"] = 5 - req.options["b"] = 10 + req.SetOption("beep", 5) + req.SetOption("b", 10) res = cmd.Call(req) - if res.Error == nil { + if res.Error() == nil { t.Error("Should have failed (duplicate options)") } req = NewEmptyRequest() - req.options["beep"] = "foo" + req.SetOption("beep", "foo") res = cmd.Call(req) - if res.Error == nil { + if res.Error() == nil { t.Error("Should have failed (incorrect type)") } req = NewEmptyRequest() - req.options["beep"] = 5 + req.SetOption("beep", 5) res = cmd.Call(req) - if res.Error != nil { - t.Error(res.Error, "Should have passed") + if res.Error() != nil { + t.Error(res.Error(), "Should have passed") } req = NewEmptyRequest() - req.options["beep"] = 5 - req.options["boop"] = "test" + req.SetOption("beep", 5) + req.SetOption("boop", "test") res = cmd.Call(req) - if res.Error != nil { + if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() - req.options["b"] = 5 - req.options["B"] = "test" + req.SetOption("b", 5) + req.SetOption("B", "test") res = cmd.Call(req) - if res.Error != nil { + if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() - req.options[EncShort] = "json" + req.SetOption(EncShort, "json") res = cmd.Call(req) - if res.Error != nil { + if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() - req.options["b"] = "100" + req.SetOption("b", "100") res = cmd.Call(req) - if res.Error != nil { + if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() - req.options["b"] = ":)" + req.SetOption("b", ":)") res = cmd.Call(req) if res.Error == nil { t.Error(res.Error, "Should have failed (string value not convertible to int)") @@ -79,40 +79,41 @@ func TestOptionValidation(t *testing.T) { } func TestRegistration(t *testing.T) { + noop := func(req Request, res Response) {} cmds := []*Command{ &Command{ Options: []Option{ Option{[]string{"beep"}, Int}, }, - run: func(req *Request, res *Response) {}, + run: noop, }, &Command{ Options: []Option{ Option{[]string{"boop"}, Int}, }, - run: func(req *Request, res *Response) {}, + run: noop, }, &Command{ Options: []Option{ Option{[]string{"boop"}, String}, }, - run: func(req *Request, res *Response) {}, + run: noop, }, &Command{ Options: []Option{ Option{[]string{"bop"}, String}, }, - run: func(req *Request, res *Response) {}, + run: noop, }, &Command{ Options: []Option{ Option{[]string{EncShort}, String}, }, - run: func(req *Request, res *Response) {}, + run: noop, }, } diff --git a/commands/request.go b/commands/request.go index b8025a6b3cd..1571a1b9332 100644 --- a/commands/request.go +++ b/commands/request.go @@ -6,37 +6,48 @@ import ( "strconv" ) +type optMap map[string]interface{} + // Request represents a call to a command from a consumer -type Request struct { +type Request interface { + Path() []string + Option(name string) (interface{}, bool) + SetOption(name string, val interface{}) + Arguments() []string + + ConvertOptions(options map[string]Option) error +} + +type request struct { path []string - options map[string]interface{} + options optMap arguments []string } -func (r *Request) Path() []string { +// Path returns the command path of this request +func (r *request) Path() []string { return r.path } -func (r *Request) SetPath(path []string) { - r.path = path -} - -func (r *Request) Option(name string) (interface{}, bool) { - val, ok := r.options[name] - return val, ok +// Option returns the value of the option for given name. +func (r *request) Option(name string) (interface{}, bool) { + val, err := r.options[name] + return val, err } -func (r *Request) SetOption(name string, value interface{}) { - r.options[name] = value +// SetOption sets the value of the option for given name. +func (r *request) SetOption(name string, val interface{}) { + r.options[name] = val } -func (r *Request) Arguments() []string { +// Arguments returns the arguments slice +func (r *request) Arguments() []string { return r.arguments } type converter func(string) (interface{}, error) -var converters map[reflect.Kind]converter = map[reflect.Kind]converter{ +var converters = map[reflect.Kind]converter{ Bool: func(v string) (interface{}, error) { if v == "" { return true, nil @@ -54,7 +65,7 @@ var converters map[reflect.Kind]converter = map[reflect.Kind]converter{ }, } -func (r *Request) convertOptions(options map[string]Option) error { +func (r *request) ConvertOptions(options map[string]Option) error { converted := make(map[string]interface{}) for k, v := range r.options { @@ -98,11 +109,13 @@ func (r *Request) convertOptions(options map[string]Option) error { return nil } -func NewEmptyRequest() *Request { +// NewEmptyRequest initializes an empty request +func NewEmptyRequest() Request { return NewRequest(nil, nil, nil) } -func NewRequest(path []string, opts map[string]interface{}, args []string) *Request { +// NewRequest returns a request initialized with given arguments +func NewRequest(path []string, opts optMap, args []string) Request { if path == nil { path = make([]string, 0) } @@ -112,5 +125,5 @@ func NewRequest(path []string, opts map[string]interface{}, args []string) *Requ if args == nil { args = make([]string, 0) } - return &Request{path, opts, args} + return &request{path, opts, args} } diff --git a/commands/response.go b/commands/response.go index c9d1c5b9302..a8b01b8db04 100644 --- a/commands/response.go +++ b/commands/response.go @@ -48,21 +48,53 @@ var marshallers = map[EncodingType]Marshaller{ // Response is the result of a command request. Handlers write to the response, // setting Error or Value. Response is returned to the client. -type Response struct { - req *Request - Error *Error - Value interface{} +type Response interface { + Request() Request + + // Set/Return the response Error + SetError(err error, code ErrorType) + Error() error + + // Sets/Returns the response value + SetValue(interface{}) + Value() interface{} + + // Marshal marshals out the response into a buffer. It uses the EncodingType + // on the Request to chose a Marshaller (Codec). + Marshal() ([]byte, error) +} + +type response struct { + req Request + err *Error + value interface{} +} + +func (r *response) Request() Request { + return r.req +} + +func (r *response) Value() interface{} { + return r.value +} + +func (r *response) SetValue(v interface{}) { + r.value = v +} + +func (r *response) Error() error { + if r.err == nil { + return nil + } + return r.err } -// SetError updates the response Error. -func (r *Response) SetError(err error, code ErrorType) { - r.Error = &Error{Message: err.Error(), Code: code} +func (r *response) SetError(err error, code ErrorType) { + r.err = &Error{Message: err.Error(), Code: code} } -// Marshal marshals out the response into a buffer. It uses the EncodingType -// on the Request to chose a Marshaller (Codec). -func (r *Response) Marshal() ([]byte, error) { - if r.Error == nil && r.Value == nil { +func (r *response) Marshal() ([]byte, error) { + if r.err == nil && r.value == nil { return nil, fmt.Errorf("No error or value set, there is nothing to marshal") } @@ -77,8 +109,13 @@ func (r *Response) Marshal() ([]byte, error) { return nil, fmt.Errorf("No marshaller found for encoding type '%s'", enc) } - if r.Error != nil { - return marshaller(r.Error) + if r.err != nil { + return marshaller(r.err) } - return marshaller(r.Value) + return marshaller(r.value) +} + +// NewResponse returns a response to match given Request +func NewResponse(req Request) Response { + return &response{req: req} } diff --git a/commands/response_test.go b/commands/response_test.go index 432d78d6978..058cead0702 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -13,10 +13,8 @@ type TestOutput struct { func TestMarshalling(t *testing.T) { req := NewEmptyRequest() - res := Response{ - req: req, - Value: TestOutput{"beep", "boop", 1337}, - } + res := NewResponse(req) + res.SetValue(TestOutput{"beep", "boop", 1337}) // get command global options so we can set the encoding option cmd := Command{} @@ -31,7 +29,7 @@ func TestMarshalling(t *testing.T) { } req.SetOption(EncShort, JSON) - req.convertOptions(options) + req.ConvertOptions(options) bytes, err := res.Marshal() if err != nil { From c0b28dc19dd50792aff5eec000ddb066b2fff9be Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 20 Oct 2014 14:38:09 -0700 Subject: [PATCH 59/73] commands: Added input stream field to Request --- commands/request.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/commands/request.go b/commands/request.go index 1571a1b9332..6259e94ba63 100644 --- a/commands/request.go +++ b/commands/request.go @@ -4,6 +4,7 @@ import ( "fmt" "reflect" "strconv" + "io" ) type optMap map[string]interface{} @@ -14,6 +15,7 @@ type Request interface { Option(name string) (interface{}, bool) SetOption(name string, val interface{}) Arguments() []string + Stream() io.Reader ConvertOptions(options map[string]Option) error } @@ -22,6 +24,7 @@ type request struct { path []string options optMap arguments []string + in io.Reader } // Path returns the command path of this request @@ -45,6 +48,11 @@ func (r *request) Arguments() []string { return r.arguments } +// Stream returns the input stream Reader +func (r *request) Stream() io.Reader { + return r.in +} + type converter func(string) (interface{}, error) var converters = map[reflect.Kind]converter{ @@ -111,11 +119,11 @@ func (r *request) ConvertOptions(options map[string]Option) error { // NewEmptyRequest initializes an empty request func NewEmptyRequest() Request { - return NewRequest(nil, nil, nil) + return NewRequest(nil, nil, nil, nil) } // NewRequest returns a request initialized with given arguments -func NewRequest(path []string, opts optMap, args []string) Request { +func NewRequest(path []string, opts optMap, args []string, in io.Reader) Request { if path == nil { path = make([]string, 0) } @@ -125,5 +133,5 @@ func NewRequest(path []string, opts optMap, args []string) Request { if args == nil { args = make([]string, 0) } - return &request{path, opts, args} + return &request{path, opts, args, in} } From 7bd7ed6d52ed694f265a7dfcc3d0db504141243c Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 20 Oct 2014 14:38:36 -0700 Subject: [PATCH 60/73] commands: Added output stream field to Response --- commands/command.go | 8 +++++--- commands/response.go | 13 +++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/commands/command.go b/commands/command.go index cf9472dc2f7..6d165d43c9b 100644 --- a/commands/command.go +++ b/commands/command.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "strings" + "io" u "github.com/jbenet/go-ipfs/util" ) @@ -46,9 +47,10 @@ func (c *Command) Register(id string, sub *Command) error { return nil } -// Call invokes the command at the given subcommand path -func (c *Command) Call(req Request) Response { - res := NewResponse(req) +// Call invokes the command for the given Request +// Streaming output is written to `out` +func (c *Command) Call(req Request, out io.Writer) Response { + res := NewResponse(req, out) cmds, err := c.Resolve(req.Path()) if err != nil { diff --git a/commands/response.go b/commands/response.go index a8b01b8db04..65f03b571ea 100644 --- a/commands/response.go +++ b/commands/response.go @@ -5,6 +5,7 @@ import ( "encoding/xml" "fmt" "strings" + "io" ) // ErrorType signfies a category of errors @@ -59,6 +60,9 @@ type Response interface { SetValue(interface{}) Value() interface{} + // Returns the output stream Writer + Stream() io.Writer + // Marshal marshals out the response into a buffer. It uses the EncodingType // on the Request to chose a Marshaller (Codec). Marshal() ([]byte, error) @@ -68,6 +72,7 @@ type response struct { req Request err *Error value interface{} + out io.Writer } func (r *response) Request() Request { @@ -82,6 +87,10 @@ func (r *response) SetValue(v interface{}) { r.value = v } +func (r *response) Stream() io.Writer { + return r.out +} + func (r *response) Error() error { if r.err == nil { return nil @@ -116,6 +125,6 @@ func (r *response) Marshal() ([]byte, error) { } // NewResponse returns a response to match given Request -func NewResponse(req Request) Response { - return &response{req: req} +func NewResponse(req Request, out io.Writer) Response { + return &response{req: req, out: out} } From b022ba4a3ae3834ebfaccf2404ee66a3597357a8 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 20 Oct 2014 14:39:23 -0700 Subject: [PATCH 61/73] commands: Fixed tests --- commands/command_test.go | 18 +++++++++--------- commands/response_test.go | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/commands/command_test.go b/commands/command_test.go index 6850fa40135..2f46d1e4292 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -13,7 +13,7 @@ func TestOptionValidation(t *testing.T) { req := NewEmptyRequest() req.SetOption("foo", 5) - res := cmd.Call(req) + res := cmd.Call(req, nil) if res.Error() == nil { t.Error("Should have failed (unrecognized option)") } @@ -21,21 +21,21 @@ func TestOptionValidation(t *testing.T) { req = NewEmptyRequest() req.SetOption("beep", 5) req.SetOption("b", 10) - res = cmd.Call(req) + res = cmd.Call(req, nil) if res.Error() == nil { t.Error("Should have failed (duplicate options)") } req = NewEmptyRequest() req.SetOption("beep", "foo") - res = cmd.Call(req) + res = cmd.Call(req, nil) if res.Error() == nil { t.Error("Should have failed (incorrect type)") } req = NewEmptyRequest() req.SetOption("beep", 5) - res = cmd.Call(req) + res = cmd.Call(req, nil) if res.Error() != nil { t.Error(res.Error(), "Should have passed") } @@ -43,7 +43,7 @@ func TestOptionValidation(t *testing.T) { req = NewEmptyRequest() req.SetOption("beep", 5) req.SetOption("boop", "test") - res = cmd.Call(req) + res = cmd.Call(req, nil) if res.Error() != nil { t.Error("Should have passed") } @@ -51,28 +51,28 @@ func TestOptionValidation(t *testing.T) { req = NewEmptyRequest() req.SetOption("b", 5) req.SetOption("B", "test") - res = cmd.Call(req) + res = cmd.Call(req, nil) if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() req.SetOption(EncShort, "json") - res = cmd.Call(req) + res = cmd.Call(req, nil) if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() req.SetOption("b", "100") - res = cmd.Call(req) + res = cmd.Call(req, nil) if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() req.SetOption("b", ":)") - res = cmd.Call(req) + res = cmd.Call(req, nil) if res.Error == nil { t.Error(res.Error, "Should have failed (string value not convertible to int)") } diff --git a/commands/response_test.go b/commands/response_test.go index 058cead0702..a0ed3356c04 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -13,7 +13,7 @@ type TestOutput struct { func TestMarshalling(t *testing.T) { req := NewEmptyRequest() - res := NewResponse(req) + res := NewResponse(req, nil) res.SetValue(TestOutput{"beep", "boop", 1337}) // get command global options so we can set the encoding option From 71ff571ecfa78568782837ecb4c5662ad5428b6f Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 20 Oct 2014 17:02:06 -0700 Subject: [PATCH 62/73] commands/cli: Made Parse return component fields instead of a Request --- commands/cli/parse.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index a953d0c5e26..5f0b2d79513 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -9,14 +9,14 @@ import ( // Parse parses the input commandline string (cmd, flags, and args). // returns the corresponding command Request object. -func Parse(input []string, root *commands.Command) (commands.Request, error) { +func Parse(input []string, root *commands.Command) ([]string, map[string]interface{}, []string, error) { path, input := parsePath(input, root) opts, args, err := parseOptions(input) if err != nil { - return nil, err + return nil, nil, nil, err } - return commands.NewRequest(path, opts, args), nil + return path, opts, args, nil } // parsePath gets the command path from the command line input From 4896123c1465fa5766c9f2c842590095e7c2681a Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 15:24:39 -0700 Subject: [PATCH 63/73] commands: Export command Run function --- commands/command.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/commands/command.go b/commands/command.go index 6d165d43c9b..d1c07dc0118 100644 --- a/commands/command.go +++ b/commands/command.go @@ -20,8 +20,8 @@ type Function func(Request, Response) type Command struct { Help string Options []Option + Run Function - run Function subcommands map[string]*Command } @@ -59,7 +59,7 @@ func (c *Command) Call(req Request, out io.Writer) Response { } cmd := cmds[len(cmds)-1] - if cmd.run == nil { + if cmd.Run == nil { res.SetError(ErrNotCallable, ErrClient) return res } @@ -76,7 +76,7 @@ func (c *Command) Call(req Request, out io.Writer) Response { return res } - cmd.run(req, res) + cmd.Run(req, res) return res } From 8786878fdb5d2eaa8ef47ed0d96fb9f813b50149 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 15:25:01 -0700 Subject: [PATCH 64/73] commands: Fixed tests --- commands/command_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/commands/command_test.go b/commands/command_test.go index 2f46d1e4292..51a447421ca 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -8,7 +8,7 @@ func TestOptionValidation(t *testing.T) { Option{[]string{"b", "beep"}, Int}, Option{[]string{"B", "boop"}, String}, }, - run: func(req Request, res Response) {}, + Run: func(req Request, res Response) {}, } req := NewEmptyRequest() @@ -85,35 +85,35 @@ func TestRegistration(t *testing.T) { Options: []Option{ Option{[]string{"beep"}, Int}, }, - run: noop, + Run: noop, }, &Command{ Options: []Option{ Option{[]string{"boop"}, Int}, }, - run: noop, + Run: noop, }, &Command{ Options: []Option{ Option{[]string{"boop"}, String}, }, - run: noop, + Run: noop, }, &Command{ Options: []Option{ Option{[]string{"bop"}, String}, }, - run: noop, + Run: noop, }, &Command{ Options: []Option{ Option{[]string{EncShort}, String}, }, - run: noop, + Run: noop, }, } From b65a5bacbe5cc8c29176f319d86bacd896c5d4f8 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 15:25:47 -0700 Subject: [PATCH 65/73] commands: Made Error implement error interface --- commands/response.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/response.go b/commands/response.go index 65f03b571ea..336134812bc 100644 --- a/commands/response.go +++ b/commands/response.go @@ -24,7 +24,7 @@ type Error struct { Code ErrorType } -func (e *Error) Error() string { +func (e Error) Error() string { return fmt.Sprintf("%d error: %s", e.Code, e.Message) } From dd84a3eb4483f34d87ccb9ea88e6fcd264030935 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 16:15:06 -0700 Subject: [PATCH 66/73] commands: Got rid of Response#Stream() in favor of setting value to a io.Reader --- commands/command.go | 6 ++---- commands/response.go | 7 ++----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/commands/command.go b/commands/command.go index d1c07dc0118..beeade91526 100644 --- a/commands/command.go +++ b/commands/command.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "strings" - "io" u "github.com/jbenet/go-ipfs/util" ) @@ -48,9 +47,8 @@ func (c *Command) Register(id string, sub *Command) error { } // Call invokes the command for the given Request -// Streaming output is written to `out` -func (c *Command) Call(req Request, out io.Writer) Response { - res := NewResponse(req, out) +func (c *Command) Call(req Request) Response { + res := NewResponse(req) cmds, err := c.Resolve(req.Path()) if err != nil { diff --git a/commands/response.go b/commands/response.go index 336134812bc..f695427099e 100644 --- a/commands/response.go +++ b/commands/response.go @@ -60,9 +60,6 @@ type Response interface { SetValue(interface{}) Value() interface{} - // Returns the output stream Writer - Stream() io.Writer - // Marshal marshals out the response into a buffer. It uses the EncodingType // on the Request to chose a Marshaller (Codec). Marshal() ([]byte, error) @@ -125,6 +122,6 @@ func (r *response) Marshal() ([]byte, error) { } // NewResponse returns a response to match given Request -func NewResponse(req Request, out io.Writer) Response { - return &response{req: req, out: out} +func NewResponse(req Request) Response { + return &response{req: req} } From 4f10f036600b42fb63805bfe00c5bee5c09d723a Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 16:15:22 -0700 Subject: [PATCH 67/73] commands: Fixed tests --- commands/command_test.go | 18 +++++++++--------- commands/response_test.go | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/commands/command_test.go b/commands/command_test.go index 51a447421ca..e8ca584e41c 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -13,7 +13,7 @@ func TestOptionValidation(t *testing.T) { req := NewEmptyRequest() req.SetOption("foo", 5) - res := cmd.Call(req, nil) + res := cmd.Call(req) if res.Error() == nil { t.Error("Should have failed (unrecognized option)") } @@ -21,21 +21,21 @@ func TestOptionValidation(t *testing.T) { req = NewEmptyRequest() req.SetOption("beep", 5) req.SetOption("b", 10) - res = cmd.Call(req, nil) + res = cmd.Call(req) if res.Error() == nil { t.Error("Should have failed (duplicate options)") } req = NewEmptyRequest() req.SetOption("beep", "foo") - res = cmd.Call(req, nil) + res = cmd.Call(req) if res.Error() == nil { t.Error("Should have failed (incorrect type)") } req = NewEmptyRequest() req.SetOption("beep", 5) - res = cmd.Call(req, nil) + res = cmd.Call(req) if res.Error() != nil { t.Error(res.Error(), "Should have passed") } @@ -43,7 +43,7 @@ func TestOptionValidation(t *testing.T) { req = NewEmptyRequest() req.SetOption("beep", 5) req.SetOption("boop", "test") - res = cmd.Call(req, nil) + res = cmd.Call(req) if res.Error() != nil { t.Error("Should have passed") } @@ -51,28 +51,28 @@ func TestOptionValidation(t *testing.T) { req = NewEmptyRequest() req.SetOption("b", 5) req.SetOption("B", "test") - res = cmd.Call(req, nil) + res = cmd.Call(req) if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() req.SetOption(EncShort, "json") - res = cmd.Call(req, nil) + res = cmd.Call(req) if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() req.SetOption("b", "100") - res = cmd.Call(req, nil) + res = cmd.Call(req) if res.Error() != nil { t.Error("Should have passed") } req = NewEmptyRequest() req.SetOption("b", ":)") - res = cmd.Call(req, nil) + res = cmd.Call(req) if res.Error == nil { t.Error(res.Error, "Should have failed (string value not convertible to int)") } diff --git a/commands/response_test.go b/commands/response_test.go index a0ed3356c04..058cead0702 100644 --- a/commands/response_test.go +++ b/commands/response_test.go @@ -13,7 +13,7 @@ type TestOutput struct { func TestMarshalling(t *testing.T) { req := NewEmptyRequest() - res := NewResponse(req, nil) + res := NewResponse(req) res.SetValue(TestOutput{"beep", "boop", 1337}) // get command global options so we can set the encoding option From 6ff98df9c10a16fff9205f3686f0b576ad0d70d8 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 17:39:30 -0700 Subject: [PATCH 68/73] commands: Do command collision check in GetOptions --- commands/command.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/commands/command.go b/commands/command.go index beeade91526..bcefc874dda 100644 --- a/commands/command.go +++ b/commands/command.go @@ -111,13 +111,13 @@ func (c *Command) Get(path []string) (*Command, error) { // GetOptions gets the options in the given path of commands func (c *Command) GetOptions(path []string) (map[string]Option, error) { options := make([]Option, len(c.Options)) - copy(options, c.Options) - options = append(options, globalOptions...) cmds, err := c.Resolve(path) if err != nil { return nil, err } + cmds = append(cmds, globalCommand) + for _, cmd := range cmds { options = append(options, cmd.Options...) } @@ -125,6 +125,10 @@ func (c *Command) GetOptions(path []string) (map[string]Option, error) { optionsMap := make(map[string]Option) for _, opt := range options { for _, name := range opt.Names { + if _, found := optionsMap[name]; found { + return nil, fmt.Errorf("Option name '%s' used multiple times", name) + } + optionsMap[name] = opt } } From ca44d0da2c4602a804f7ca67fa7a4e8e71edab5e Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 18:05:54 -0700 Subject: [PATCH 69/73] commands: Removed Command#Register and exported Subcommands so subcommands can be defined statically --- commands/command.go | 75 ++++----------------------------------------- 1 file changed, 6 insertions(+), 69 deletions(-) diff --git a/commands/command.go b/commands/command.go index bcefc874dda..b01a579e74b 100644 --- a/commands/command.go +++ b/commands/command.go @@ -15,37 +15,17 @@ var log = u.Logger("command") type Function func(Request, Response) // Command is a runnable command, with input arguments and options (flags). -// It can also have subcommands, to group units of work into sets. +// It can also have Subcommands, to group units of work into sets. type Command struct { - Help string - Options []Option - Run Function - - subcommands map[string]*Command + Help string + Options []Option + Run Function + Subcommands map[string]*Command } // ErrNotCallable signals a command that cannot be called. var ErrNotCallable = errors.New("This command can't be called directly. Try one of its subcommands.") -// Register adds a subcommand -func (c *Command) Register(id string, sub *Command) error { - if c.subcommands == nil { - c.subcommands = make(map[string]*Command) - } - - // check for duplicate option names (only checks downwards) - if err := checkOptionClashes(globalCommand, c, sub); err != nil { - return err - } - - if _, found := c.subcommands[id]; found { - return fmt.Errorf("There is already a subcommand registered with id '%s'", id) - } - - c.subcommands[id] = sub - return nil -} - // Call invokes the command for the given Request func (c *Command) Call(req Request) Response { res := NewResponse(req) @@ -138,48 +118,5 @@ func (c *Command) GetOptions(path []string) (map[string]Option, error) { // Subcommand returns the subcommand with the given id func (c *Command) Subcommand(id string) *Command { - return c.subcommands[id] -} - -// AddOptionNames returns a map of all command options names, and the command -// they belong to. Will error if names clash in the command hierarchy. -func AddOptionNames(c *Command, names map[string]*Command) error { - - for _, opt := range c.Options { - for _, name := range opt.Names { - if c2, found := names[name]; found { - - // option can be reused in same command, but more often than not - // the clash will be across commands so error out with that, as - // commands tell us where the problem is - errstr := "Option name ('%s') used multiple times (%v, %v)" - return fmt.Errorf(errstr, c2, c) - } - - // mark the name as in use - names[name] = c - } - } - - // for every subcommand, recurse - for _, c2 := range c.subcommands { - if err := AddOptionNames(c2, names); err != nil { - return err - } - } - - return nil -} - -// checkOptionClashes checks all command option names for clashes -func checkOptionClashes(cmds ...*Command) error { - names := map[string]*Command{} - - for _, c := range cmds { - if err := AddOptionNames(c, names); err != nil { - return err - } - } - - return nil + return c.Subcommands[id] } From dd81bf630fe616d7068aae82d9150d1db31b40e3 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 18:06:19 -0700 Subject: [PATCH 70/73] commands: Fixed tests --- commands/cli/parse_test.go | 4 +- commands/command_test.go | 88 ++++++++++++++++---------------------- 2 files changed, 40 insertions(+), 52 deletions(-) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index dffc4b1430c..1c5e1ff413d 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -12,8 +12,10 @@ func TestOptionParsing(t *testing.T) { Options: []commands.Option{ commands.Option{Names: []string{"b"}, Type: commands.String}, }, + Subcommands: map[string]*commands.Command{ + "test": &commands.Command{}, + }, } - cmd.Register("test", &commands.Command{}) opts, input, err := parseOptions([]string{"--beep", "-boop=lol", "test2", "-c", "beep", "--foo=5"}) /*for k, v := range opts { diff --git a/commands/command_test.go b/commands/command_test.go index e8ca584e41c..63a3112be66 100644 --- a/commands/command_test.go +++ b/commands/command_test.go @@ -80,75 +80,61 @@ func TestOptionValidation(t *testing.T) { func TestRegistration(t *testing.T) { noop := func(req Request, res Response) {} - cmds := []*Command{ - &Command{ - Options: []Option{ - Option{[]string{"beep"}, Int}, - }, - Run: noop, - }, - - &Command{ - Options: []Option{ - Option{[]string{"boop"}, Int}, - }, - Run: noop, - }, - &Command{ - Options: []Option{ - Option{[]string{"boop"}, String}, - }, - Run: noop, + cmdA := &Command{ + Options: []Option{ + Option{[]string{"beep"}, Int}, }, + Run: noop, + } - &Command{ - Options: []Option{ - Option{[]string{"bop"}, String}, - }, - Run: noop, + cmdB := &Command{ + Options: []Option{ + Option{[]string{"beep"}, Int}, }, - - &Command{ - Options: []Option{ - Option{[]string{EncShort}, String}, - }, - Run: noop, + Run: noop, + Subcommands: map[string]*Command{ + "a": cmdA, }, } - err := cmds[0].Register("foo", cmds[1]) - if err != nil { - t.Error("Should have passed") + cmdC := &Command{ + Options: []Option{ + Option{[]string{"encoding"}, String}, + }, + Run: noop, } - err = cmds[0].Register("bar", cmds[2]) - if err == nil { + res := cmdB.Call(NewRequest([]string{"a"}, nil, nil, nil)) + if res.Error() == nil { t.Error("Should have failed (option name collision)") } - err = cmds[0].Register("foo", cmds[3]) - if err == nil { - t.Error("Should have failed (subcommand name collision)") - } - - err = cmds[0].Register("baz", cmds[4]) - if err == nil { + res = cmdC.Call(NewEmptyRequest()) + if res.Error() == nil { t.Error("Should have failed (option name collision with global options)") } } func TestResolving(t *testing.T) { - cmd := &Command{} - cmdA := &Command{} - cmdB := &Command{} - cmdB2 := &Command{} cmdC := &Command{} - - cmd.Register("a", cmdA) - cmdA.Register("B", cmdB2) - cmdA.Register("b", cmdB) - cmdB.Register("c", cmdC) + cmdB := &Command{ + Subcommands: map[string]*Command{ + "c": cmdC, + }, + } + cmdB2 := &Command{} + cmdA := &Command{ + Subcommands: map[string]*Command{ + "b": cmdB, + "B": cmdB2, + }, + } + cmd := &Command{ + Subcommands: map[string]*Command{ + "a": cmdA, + }, + } cmds, err := cmd.Resolve([]string{"a", "b", "c"}) if err != nil { From d464e3d143ee4d56cfad9849b7392834bb0b0a3d Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Tue, 21 Oct 2014 16:29:08 -0700 Subject: [PATCH 71/73] commands: go fmt --- commands/request.go | 4 ++-- commands/response.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/commands/request.go b/commands/request.go index 6259e94ba63..04add77aeb1 100644 --- a/commands/request.go +++ b/commands/request.go @@ -2,9 +2,9 @@ package commands import ( "fmt" + "io" "reflect" "strconv" - "io" ) type optMap map[string]interface{} @@ -24,7 +24,7 @@ type request struct { path []string options optMap arguments []string - in io.Reader + in io.Reader } // Path returns the command path of this request diff --git a/commands/response.go b/commands/response.go index f695427099e..61dede6e2f2 100644 --- a/commands/response.go +++ b/commands/response.go @@ -4,8 +4,8 @@ import ( "encoding/json" "encoding/xml" "fmt" + "io" "strings" - "io" ) // ErrorType signfies a category of errors @@ -69,7 +69,7 @@ type response struct { req Request err *Error value interface{} - out io.Writer + out io.Writer } func (r *response) Request() Request { @@ -85,7 +85,7 @@ func (r *response) SetValue(v interface{}) { } func (r *response) Stream() io.Writer { - return r.out + return r.out } func (r *response) Error() error { From 12a6a87b2c8a23ebace8034f4345ff65034d52d2 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 18:27:05 -0700 Subject: [PATCH 72/73] commands/cli: Made Parse return a Request (again) --- commands/cli/parse.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 5f0b2d79513..08df7955bad 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -9,14 +9,14 @@ import ( // Parse parses the input commandline string (cmd, flags, and args). // returns the corresponding command Request object. -func Parse(input []string, root *commands.Command) ([]string, map[string]interface{}, []string, error) { +func Parse(input []string, root *commands.Command) (commands.Request, error) { path, input := parsePath(input, root) opts, args, err := parseOptions(input) if err != nil { - return nil, nil, nil, err + return nil, err } - return path, opts, args, nil + return commands.NewRequest(path, opts, args, nil), nil } // parsePath gets the command path from the command line input From 4303dccee93987fbf36f6df390de950eb7b76b0b Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Tue, 21 Oct 2014 18:27:39 -0700 Subject: [PATCH 73/73] commands: Added Request#SetStream --- commands/request.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/commands/request.go b/commands/request.go index 04add77aeb1..ab6c0dc2b5b 100644 --- a/commands/request.go +++ b/commands/request.go @@ -16,6 +16,7 @@ type Request interface { SetOption(name string, val interface{}) Arguments() []string Stream() io.Reader + SetStream(io.Reader) ConvertOptions(options map[string]Option) error } @@ -53,6 +54,11 @@ func (r *request) Stream() io.Reader { return r.in } +// SetStream sets the value of the input stream Reader +func (r *request) SetStream(in io.Reader) { + r.in = in +} + type converter func(string) (interface{}, error) var converters = map[reflect.Kind]converter{