Extract client flags specs to seperate methods

Make sure the main function is not overhelming,
extract portion of lines to separate methods for better read.

Found that client in `toxic add` supports both: `--upstream` and
`--downstream`. Because it tries to create 2 toxic with single name it
produces the error when tries add `--downstream` toxic, and in same time
creates the `--upstream` toxic.

To remove confusion update the client usage help to support only one in
same time. Return error message in case 2 flags provided.

cli/cli.go

@@ -51,7 +51,7 @@ var toxicDescription = `
 
   toxic add:
     usage: toxiproxy-cli toxic add --type <toxicType> --toxicName <toxicName> \
-            --attribute <key=value> --upstream --downstream <proxyName>
+            --attribute <key=value> [--downstream|--upstream] <proxyName>
 
     example: toxiproxy-cli toxic add -t latency -n myToxic -a latency=100 -a jitter=50 myProxy
 
@@ -77,7 +77,29 @@ func main() {
 	app.Name = "toxiproxy-cli"
 	app.Version = toxiproxyServer.Version
 	app.Usage = "Simulate network and system conditions"
-	app.Commands = []*cli.Command{
+	app.Commands = cliCommands()
+	cli.HelpFlag = &cli.BoolFlag{
+		Name:  "help",
+		Usage: "show help",
+	}
+	app.Flags = []cli.Flag{
+		&cli.StringFlag{
+			Name:        "host",
+			Aliases:     []string{"h"},
+			Value:       "http://localhost:8474",
+			Usage:       "toxiproxy host to connect to",
+			Destination: &hostname,
+			EnvVars:     []string{"TOXIPROXY_URL"},
+		},
+	}
+
+	isTTY = terminal.IsTerminal(int(os.Stdout.Fd()))
+
+	app.Run(os.Args)
+}
+
+func cliCommands() []*cli.Command {
+	return []*cli.Command{
 		{
 			Name:    "list",
 			Usage:   "list all proxies\n\tusage: 'toxiproxy-cli list'\n",
@@ -126,105 +148,105 @@ func main() {
 			Aliases:     []string{"t"},
 			Usage:       "\tadd, remove or update a toxic\n\t\tusage: see 'toxiproxy-cli toxic'\n",
 			Description: toxicDescription,
-			Subcommands: []*cli.Command{
-				{
-					Name:      "add",
-					Aliases:   []string{"a"},
-					Usage:     "add a new toxic",
-					ArgsUsage: "<proxyName>",
-					Flags: []cli.Flag{
-						&cli.StringFlag{
-							Name:    "toxicName",
-							Aliases: []string{"n"},
-							Usage:   "name of the toxic",
-						},
-						&cli.StringFlag{
-							Name:    "type",
-							Aliases: []string{"t"},
-							Usage:   "type of toxic",
-						},
-						&cli.StringFlag{
-							Name:    "toxicity",
-							Aliases: []string{"tox"},
-							Usage:   "toxicity of toxic",
-						},
-						&cli.StringSliceFlag{
-							Name:    "attribute",
-							Aliases: []string{"a"},
-							Usage:   "toxic attribute in key=value format",
-						},
-						&cli.BoolFlag{
-							Name:    "upstream",
-							Aliases: []string{"u"},
-							Usage:   "add toxic to upstream",
-						},
-						&cli.BoolFlag{
-							Name:    "downstream",
-							Aliases: []string{"d"},
-							Usage:   "add toxic to downstream",
-						},
-					},
-					Action: withToxi(addToxic),
-				},
-				{
-					Name:      "update",
-					Aliases:   []string{"u"},
-					Usage:     "update an enabled toxic",
-					ArgsUsage: "<proxyName>",
-					Flags: []cli.Flag{
-						&cli.StringFlag{
-							Name:    "toxicName",
-							Aliases: []string{"n"},
-							Usage:   "name of the toxic",
-						},
-						&cli.StringFlag{
-							Name:    "toxicity",
-							Aliases: []string{"tox"},
-							Usage:   "toxicity of toxic",
-						},
-						&cli.StringSliceFlag{
-							Name:    "attribute",
-							Aliases: []string{"a"},
-							Usage:   "toxic attribute in key=value format",
-						},
-					},
-					Action: withToxi(updateToxic),
-				},
-				{
-					Name:      "remove",
-					Aliases:   []string{"r", "delete", "d"},
-					Usage:     "remove an enabled toxic",
-					ArgsUsage: "<proxyName>",
-					Flags: []cli.Flag{
-						&cli.StringFlag{
-							Name:    "toxicName",
-							Aliases: []string{"n"},
-							Usage:   "name of the toxic",
-						},
-					},
-					Action: withToxi(removeToxic),
-				},
-			},
+			Subcommands: cliToxiSubCommands(),
 		},
 	}
-	cli.HelpFlag = &cli.BoolFlag{
-		Name:  "help",
-		Usage: "show help",
+}
+
+func cliToxiSubCommands() []*cli.Command {
+	return []*cli.Command{
+		cliToxiAddSubCommand(),
+		cliToxiUpdateSubCommand(),
+		cliToxiRemoveSubCommand(),
 	}
-	app.Flags = []cli.Flag{
-		&cli.StringFlag{
-			Name:        "host",
-			Aliases:     []string{"h"},
-			Value:       "http://localhost:8474",
-			Usage:       "toxiproxy host to connect to",
-			Destination: &hostname,
-			EnvVars:     []string{"TOXIPROXY_URL"},
+}
+
+func cliToxiAddSubCommand() *cli.Command {
+	return &cli.Command{
+		Name:      "add",
+		Aliases:   []string{"a"},
+		Usage:     "add a new toxic",
+		ArgsUsage: "<proxyName>",
+		Flags: []cli.Flag{
+			&cli.StringFlag{
+				Name:    "toxicName",
+				Aliases: []string{"n"},
+				Usage:   "name of the toxic",
+			},
+			&cli.StringFlag{
+				Name:    "type",
+				Aliases: []string{"t"},
+				Usage:   "type of toxic",
+			},
+			&cli.StringFlag{
+				Name:    "toxicity",
+				Aliases: []string{"tox"},
+				Usage:   "toxicity of toxic",
+			},
+			&cli.StringSliceFlag{
+				Name:    "attribute",
+				Aliases: []string{"a"},
+				Usage:   "toxic attribute in key=value format",
+			},
+			&cli.BoolFlag{
+				Name:        "upstream",
+				Aliases:     []string{"u"},
+				Usage:       "add toxic to upstream",
+				DefaultText: "false",
+			},
+			&cli.BoolFlag{
+				Name:        "downstream",
+				Aliases:     []string{"d"},
+				Usage:       "add toxic to downstream",
+				DefaultText: "true",
+			},
 		},
+		Action: withToxi(addToxic),
 	}
+}
 
-	isTTY = terminal.IsTerminal(int(os.Stdout.Fd()))
+func cliToxiUpdateSubCommand() *cli.Command {
+	return &cli.Command{
+		Name:      "update",
+		Aliases:   []string{"u"},
+		Usage:     "update an enabled toxic",
+		ArgsUsage: "<proxyName>",
+		Flags: []cli.Flag{
+			&cli.StringFlag{
+				Name:    "toxicName",
+				Aliases: []string{"n"},
+				Usage:   "name of the toxic",
+			},
+			&cli.StringFlag{
+				Name:    "toxicity",
+				Aliases: []string{"tox"},
+				Usage:   "toxicity of toxic",
+			},
+			&cli.StringSliceFlag{
+				Name:    "attribute",
+				Aliases: []string{"a"},
+				Usage:   "toxic attribute in key=value format",
+			},
+		},
+		Action: withToxi(updateToxic),
+	}
+}
 
-	app.Run(os.Args)
+func cliToxiRemoveSubCommand() *cli.Command {
+	return &cli.Command{
+		Name:      "remove",
+		Aliases:   []string{"r", "delete", "d"},
+		Usage:     "remove an enabled toxic",
+		ArgsUsage: "<proxyName>",
+		Flags: []cli.Flag{
+			&cli.StringFlag{
+				Name:    "toxicName",
+				Aliases: []string{"n"},
+				Usage:   "name of the toxic",
+			},
+		},
+		Action: withToxi(removeToxic),
+	}
 }
 
 type toxiAction func(*cli.Context, *toxiproxy.Client) error
@@ -421,59 +443,66 @@ func parseToxicity(c *cli.Context, defaultToxicity float32) (float32, error) {
 	return toxicity, nil
 }
 
-func addToxic(c *cli.Context, t *toxiproxy.Client) error {
+func parseAddToxicParams(c *cli.Context) (*toxiproxy.ToxicOptions, error) {
 	proxyName := c.Args().First()
 	if proxyName == "" {
 		cli.ShowSubcommandHelp(c)
-		return errorf("Proxy name is required as the first argument.\n")
+		return nil, errorf("Proxy name is missing.\n")
 	}
+
 	toxicName := c.String("toxicName")
 	toxicType, err := getArgOrFail(c, "type")
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	upstream := c.Bool("upstream")
 	downstream := c.Bool("downstream")
+	if upstream && downstream {
+		return nil, errorf("Only one should be specified: upstream or downstream.\n")
+	}
+
+	stream := "downstream"
+	if upstream {
+		stream = "upstream"
+	}
 
 	toxicity, err := parseToxicity(c, 1.0)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	attributes := parseAttributes(c, "attribute")
 
-	p, err := t.Proxy(proxyName)
+	return &toxiproxy.ToxicOptions{
+		ProxyName:  proxyName,
+		ToxicName:  toxicName,
+		ToxicType:  toxicType,
+		Toxicity:   toxicity,
+		Attributes: attributes,
+		Stream:     stream,
+	}, nil
+}
+
+func addToxic(c *cli.Context, t *toxiproxy.Client) error {
+	toxicParams, err := parseAddToxicParams(c)
 	if err != nil {
-		return errorf("Failed to retrieve proxy %s: %s\n", proxyName, err.Error())
+		return err
 	}
 
-	addToxic := func(stream string) error {
-		t, err := p.AddToxic(toxicName, toxicType, stream, toxicity, attributes)
-		if err != nil {
-			return errorf("Failed to add toxic: %s\n", err.Error())
-		}
-		toxicName = t.Name
-		fmt.Printf(
-			"Added %s %s toxic '%s' on proxy '%s'\n",
-			stream,
-			toxicType,
-			toxicName,
-			proxyName,
-		)
-		return nil
+	toxic, err := t.AddToxic(toxicParams)
+	if err != nil {
+		return errorf("Failed to add toxic: %v\n", err)
 	}
 
-	if upstream {
-		err := addToxic("upstream")
-		if err != nil {
-			return err
-		}
-	}
-	// Default to downstream.
-	if downstream || (!downstream && !upstream) {
-		return addToxic("downstream")
-	}
+	fmt.Printf(
+		"Added %s %s toxic '%s' on proxy '%s'\n",
+		toxic.Stream,
+		toxic.Type,
+		toxic.Name,
+		toxicParams.ProxyName,
+	)
+
 	return nil
 }
 

client/README.md

@@ -32,7 +32,7 @@ documentation](http://godoc.org/github.com/Shopify/toxiproxy/client).
 
 First import toxiproxy and create a new client:
 ```go
-import "github.com/Shopify/toxiproxy/client"
+import toxiproxy "github.com/Shopify/toxiproxy/v2/client"
 
 client := toxiproxy.NewClient("localhost:8474")
 ```
@@ -95,7 +95,7 @@ import (
     "testing"
     "time"
 
-    "github.com/Shopify/toxiproxy/client"
+    toxiproxy "github.com/Shopify/toxiproxy/v2/client"
     "github.com/garyburd/redigo/redis"
 )
 

client/client.go

@@ -167,6 +167,28 @@ func (client *Client) Populate(config []Proxy) ([]*Proxy, error) {
 	return proxies.Proxies, err
 }
 
+// AddToxic creates a toxic to proxy
+func (client *Client) AddToxic(options *ToxicOptions) (*Toxic, error) {
+	proxy, err := client.Proxy(options.ProxyName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to retrieve proxy with name `%s`: %v", options.ProxyName, err)
+	}
+
+	toxic, err := proxy.AddToxic(
+		options.ToxicName,
+		options.ToxicType,
+		options.Stream,
+		options.Toxicity,
+		options.Attributes,
+	)
+
+	if err != nil {
+		return nil, fmt.Errorf("failed to add toxic to proxy %s: %v", options.ProxyName, err)
+	}
+
+	return toxic, nil
+}
+
 // Save saves changes to a proxy such as its enabled status or upstream port.
 func (proxy *Proxy) Save() error {
 	request, err := json.Marshal(proxy)

client/toxic_option.go

@@ -0,0 +1,10 @@
+package toxiproxy
+
+type ToxicOptions struct {
+	ProxyName,
+	ToxicName,
+	ToxicType,
+	Stream string
+	Toxicity   float32
+	Attributes Attributes
+}