From 7b0370b0352a5f395d57a9c0c2ac39e7fb01a07b Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Mon, 22 Apr 2024 13:53:43 -0700 Subject: [PATCH 1/3] Create an --output/-o flag for JSON or plain text responses --- cmd/cliflags/flags.go | 1 + cmd/config/testdata/help.golden | 1 + cmd/root.go | 11 +++++++++++ 3 files changed, 13 insertions(+) diff --git a/cmd/cliflags/flags.go b/cmd/cliflags/flags.go index 73b95e38..d7508e89 100644 --- a/cmd/cliflags/flags.go +++ b/cmd/cliflags/flags.go @@ -7,6 +7,7 @@ const ( EmailsFlag = "emails" EnvironmentFlag = "environment" FlagFlag = "flag" + OutputFlag = "output" ProjectFlag = "project" RoleFlag = "role" ) diff --git a/cmd/config/testdata/help.golden b/cmd/config/testdata/help.golden index 0862a0f9..98a62fb6 100644 --- a/cmd/config/testdata/help.golden +++ b/cmd/config/testdata/help.golden @@ -12,3 +12,4 @@ Flags: Global Flags: --access-token string LaunchDarkly API token with write-level access --base-uri string LaunchDarkly base URI (default "https://app.launchdarkly.com") + -o, --output string Command response output format in either JSON or plain text (default "plaintext") diff --git a/cmd/root.go b/cmd/root.go index cb9c0020..d49e5bea 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -96,6 +96,17 @@ func NewRootCommand( return nil, err } + cmd.PersistentFlags().StringP( + cliflags.OutputFlag, + "o", + "plaintext", + "Command response output format in either JSON or plain text", + ) + err = viper.BindPFlag(cliflags.OutputFlag, cmd.PersistentFlags().Lookup(cliflags.OutputFlag)) + if err != nil { + return nil, err + } + environmentsCmd, err := envscmd.NewEnvironmentsCmd(analyticsTracker, clients.EnvironmentsClient) if err != nil { return nil, err From fb9887f559e887f97285e1246724649aa28264d7 Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Mon, 22 Apr 2024 14:33:56 -0700 Subject: [PATCH 2/3] Refactor command error handling --- cmd/environments/get_test.go | 17 +++++++++++++++ cmd/validators/validators.go | 41 +++++++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/cmd/environments/get_test.go b/cmd/environments/get_test.go index 50cb0bf0..d7c2a8c5 100644 --- a/cmd/environments/get_test.go +++ b/cmd/environments/get_test.go @@ -142,4 +142,21 @@ func TestGet(t *testing.T) { assert.EqualError(t, err, "base-uri is invalid"+errorHelp) }) + + t.Run("with invalid output is an error", func(t *testing.T) { + clients := cmd.APIClients{ + EnvironmentsClient: &environments.MockClient{}, + } + args := []string{ + "environments", "get", + "--access-token", "testAccessToken", + "--output", "invalid", + "--environment", "test-env", + "--project", "test-proj", + } + + _, err := cmd.CallCmd(t, clients, args) + + assert.EqualError(t, err, "output is invalid"+errorHelp) + }) } diff --git a/cmd/validators/validators.go b/cmd/validators/validators.go index 7e96c79a..63ea0342 100644 --- a/cmd/validators/validators.go +++ b/cmd/validators/validators.go @@ -21,29 +21,33 @@ func Validate() cobra.PositionalArgs { _, err := url.ParseRequestURI(viper.GetString(cliflags.BaseURIFlag)) if err != nil { - errorMessage := fmt.Sprintf( - "%s. See `%s --help` for supported flags and usage.", - errs.ErrInvalidBaseURI, - commandPath, - ) - return errors.New(errorMessage) + return CmdError(errs.ErrInvalidBaseURI, commandPath) } err = cmd.ValidateRequiredFlags() if err != nil { - errorMessage := fmt.Sprintf( - "%s. See `%s --help` for supported flags and usage.", - err.Error(), - commandPath, - ) + return CmdError(err, commandPath) + } - return errors.New(errorMessage) + err = validateOutput(viper.GetString(cliflags.OutputFlag)) + if err != nil { + return CmdError(err, commandPath) } return nil } } +func CmdError(err error, commandPath string) error { + errorMessage := fmt.Sprintf( + "%s. See `%s --help` for supported flags and usage.", + err.Error(), + commandPath, + ) + + return errors.New(errorMessage) +} + func getCommandPath(cmd *cobra.Command) string { var commandPath string if cmd.Annotations["scope"] == "plugin" { @@ -55,6 +59,19 @@ func getCommandPath(cmd *cobra.Command) string { return commandPath } +func validateOutput(outputFlag string) error { + validKinds := map[string]struct{}{ + "json": {}, + "plaintext": {}, + } + _, ok := validKinds[outputFlag] + if !ok { + return errors.New("output is invalid") + } + + return nil +} + // rebindFlags sets the command's flags based on the values stored in viper because they may not // be set yet when they (the flags) are set from environment variables or a configuration file. func rebindFlags(cmd *cobra.Command, _ []string) { From 424d2fd192bd1e099fef7a55b9f386d0c4c38e23 Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Tue, 23 Apr 2024 11:24:50 -0700 Subject: [PATCH 3/3] feat: output flag plaintext (#197) * Use output flag to show JSON or plaintext * Call outputter in command handler * Backfill test * Rename * Refactor to pass in fn instead of interface * Remove stripe code * Create additional interface * Implement plaintext for multiple resource response (#199) --- cmd/environments/get.go | 11 +++- cmd/environments/get_test.go | 14 +++-- cmd/projects/list.go | 11 +++- cmd/projects/list_test.go | 18 ++++-- cmd/validators/validators.go | 21 ++----- internal/environments/client.go | 7 ++- internal/output/multiple_outputter.go | 49 ++++++++++++++++ internal/output/multiple_outputter_test.go | 59 +++++++++++++++++++ internal/output/output.go | 67 ++++++++++++++++++++++ internal/output/singular_outputter.go | 49 ++++++++++++++++ internal/output/singular_outputter_test.go | 41 +++++++++++++ 11 files changed, 317 insertions(+), 30 deletions(-) create mode 100644 internal/output/multiple_outputter.go create mode 100644 internal/output/multiple_outputter_test.go create mode 100644 internal/output/output.go create mode 100644 internal/output/singular_outputter.go create mode 100644 internal/output/singular_outputter_test.go diff --git a/cmd/environments/get.go b/cmd/environments/get.go index 1f633e9b..933f1c1c 100644 --- a/cmd/environments/get.go +++ b/cmd/environments/get.go @@ -11,6 +11,7 @@ import ( "ldcli/cmd/validators" "ldcli/internal/analytics" "ldcli/internal/environments" + "ldcli/internal/output" ) func NewGetCmd( @@ -68,6 +69,14 @@ func runGet( return err } + output, err := output.CmdOutput( + viper.GetString(cliflags.OutputFlag), + output.NewSingularOutputterFn(response), + ) + if err != nil { + return err + } + analyticsTracker.SendEvent( viper.GetString(cliflags.AccessTokenFlag), viper.GetString(cliflags.BaseURIFlag), @@ -77,7 +86,7 @@ func runGet( "projectKey": viper.GetString(cliflags.ProjectFlag), }) - fmt.Fprintf(cmd.OutOrStdout(), string(response)+"\n") + fmt.Fprintf(cmd.OutOrStdout(), string(output)+"\n") return nil } diff --git a/cmd/environments/get_test.go b/cmd/environments/get_test.go index d7c2a8c5..3971ab35 100644 --- a/cmd/environments/get_test.go +++ b/cmd/environments/get_test.go @@ -19,12 +19,13 @@ func TestGet(t *testing.T) { "test-env", "test-proj", } + stubbedResponse := `{"key": "test-key", "name": "test-name"}` - t.Run("with valid environments calls API", func(t *testing.T) { + t.Run("with valid flags calls API", func(t *testing.T) { client := environments.MockClient{} client. On("Get", mockArgs...). - Return([]byte(cmd.ValidResponse), nil) + Return([]byte(stubbedResponse), nil) clients := cmd.APIClients{ EnvironmentsClient: &client, } @@ -32,6 +33,7 @@ func TestGet(t *testing.T) { "environments", "get", "--access-token", "testAccessToken", "--base-uri", "http://test.com", + "--output", "json", "--environment", "test-env", "--project", "test-proj", } @@ -39,7 +41,7 @@ func TestGet(t *testing.T) { output, err := cmd.CallCmd(t, clients, args) require.NoError(t, err) - assert.JSONEq(t, `{"valid": true}`, string(output)) + assert.JSONEq(t, stubbedResponse, string(output)) }) t.Run("with valid flags from environment variables calls API", func(t *testing.T) { @@ -48,12 +50,13 @@ func TestGet(t *testing.T) { client := environments.MockClient{} client. On("Get", mockArgs...). - Return([]byte(cmd.ValidResponse), nil) + Return([]byte(stubbedResponse), nil) clients := cmd.APIClients{ EnvironmentsClient: &client, } args := []string{ "environments", "get", + "--output", "json", "--environment", "test-env", "--project", "test-proj", } @@ -61,7 +64,7 @@ func TestGet(t *testing.T) { output, err := cmd.CallCmd(t, clients, args) require.NoError(t, err) - assert.JSONEq(t, `{"valid": true}`, string(output)) + assert.JSONEq(t, stubbedResponse, string(output)) }) t.Run("with an error response is an error", func(t *testing.T) { @@ -76,6 +79,7 @@ func TestGet(t *testing.T) { "environments", "get", "--access-token", "testAccessToken", "--base-uri", "http://test.com", + "--output", "json", "--environment", "test-env", "--project", "test-proj", } diff --git a/cmd/projects/list.go b/cmd/projects/list.go index 72104faa..5cd2f874 100644 --- a/cmd/projects/list.go +++ b/cmd/projects/list.go @@ -9,6 +9,7 @@ import ( "ldcli/cmd/cliflags" "ldcli/cmd/validators" + "ldcli/internal/output" "ldcli/internal/projects" ) @@ -35,7 +36,15 @@ func runList(client projects.Client) func(*cobra.Command, []string) error { return err } - fmt.Fprintf(cmd.OutOrStdout(), string(response)+"\n") + output, err := output.CmdOutput( + viper.GetString(cliflags.OutputFlag), + output.NewMultipleOutputterFn(response), + ) + if err != nil { + return err + } + + fmt.Fprintf(cmd.OutOrStdout(), string(output)+"\n") return nil } diff --git a/cmd/projects/list_test.go b/cmd/projects/list_test.go index cf2c55ec..b468417d 100644 --- a/cmd/projects/list_test.go +++ b/cmd/projects/list_test.go @@ -17,12 +17,20 @@ func TestList(t *testing.T) { "testAccessToken", "http://test.com", } + stubbedResponse := `{ + "items": [ + { + "key": "test-key", + "name": "test-name" + } + ] + }` t.Run("with valid flags calls API", func(t *testing.T) { client := projects.MockClient{} client. On("List", mockArgs...). - Return([]byte(cmd.ValidResponse), nil) + Return([]byte(stubbedResponse), nil) clients := cmd.APIClients{ ProjectsClient: &client, } @@ -30,12 +38,13 @@ func TestList(t *testing.T) { "projects", "list", "--access-token", "testAccessToken", "--base-uri", "http://test.com", + "--output", "json", } output, err := cmd.CallCmd(t, clients, args) require.NoError(t, err) - assert.JSONEq(t, `{"valid": true}`, string(output)) + assert.JSONEq(t, stubbedResponse, string(output)) }) t.Run("with valid flags from environment variables calls API", func(t *testing.T) { @@ -44,19 +53,20 @@ func TestList(t *testing.T) { client := projects.MockClient{} client. On("List", mockArgs...). - Return([]byte(cmd.ValidResponse), nil) + Return([]byte(stubbedResponse), nil) clients := cmd.APIClients{ ProjectsClient: &client, } args := []string{ "projects", "list", + "--output", "json", } output, err := cmd.CallCmd(t, clients, args) require.NoError(t, err) - assert.JSONEq(t, `{"valid": true}`, string(output)) + assert.JSONEq(t, stubbedResponse, string(output)) }) t.Run("with an error response is an error", func(t *testing.T) { diff --git a/cmd/validators/validators.go b/cmd/validators/validators.go index 63ea0342..b1697e4f 100644 --- a/cmd/validators/validators.go +++ b/cmd/validators/validators.go @@ -11,27 +11,27 @@ import ( "ldcli/cmd/cliflags" errs "ldcli/internal/errors" + "ldcli/internal/output" ) // Validate is a validator for commands to print an error when the user input is invalid. func Validate() cobra.PositionalArgs { return func(cmd *cobra.Command, args []string) error { rebindFlags(cmd, cmd.ValidArgs) // rebind flags before validating them below - commandPath := getCommandPath(cmd) _, err := url.ParseRequestURI(viper.GetString(cliflags.BaseURIFlag)) if err != nil { - return CmdError(errs.ErrInvalidBaseURI, commandPath) + return CmdError(errs.ErrInvalidBaseURI, cmd.CommandPath()) } err = cmd.ValidateRequiredFlags() if err != nil { - return CmdError(err, commandPath) + return CmdError(err, cmd.CommandPath()) } err = validateOutput(viper.GetString(cliflags.OutputFlag)) if err != nil { - return CmdError(err, commandPath) + return CmdError(err, cmd.CommandPath()) } return nil @@ -48,17 +48,6 @@ func CmdError(err error, commandPath string) error { return errors.New(errorMessage) } -func getCommandPath(cmd *cobra.Command) string { - var commandPath string - if cmd.Annotations["scope"] == "plugin" { - commandPath = fmt.Sprintf("stripe %s", cmd.CommandPath()) - } else { - commandPath = cmd.CommandPath() - } - - return commandPath -} - func validateOutput(outputFlag string) error { validKinds := map[string]struct{}{ "json": {}, @@ -66,7 +55,7 @@ func validateOutput(outputFlag string) error { } _, ok := validKinds[outputFlag] if !ok { - return errors.New("output is invalid") + return output.ErrInvalidOutputKind } return nil diff --git a/internal/environments/client.go b/internal/environments/client.go index 39af1d41..49b84991 100644 --- a/internal/environments/client.go +++ b/internal/environments/client.go @@ -38,10 +38,11 @@ func (c EnvironmentsClient) Get( } - responseJSON, err := json.Marshal(environment) + output, err := json.Marshal(environment) if err != nil { - return nil, err + return nil, errors.NewLDAPIError(err) + } - return responseJSON, nil + return output, nil } diff --git a/internal/output/multiple_outputter.go b/internal/output/multiple_outputter.go new file mode 100644 index 00000000..97e24505 --- /dev/null +++ b/internal/output/multiple_outputter.go @@ -0,0 +1,49 @@ +package output + +import ( + "encoding/json" + "fmt" +) + +var multiplePlaintextOutputFn = func(r resource) string { + return fmt.Sprintf("* %s (%s)", r.Name, r.Key) +} + +// TODO: rename this to be "cleaner"? -- NewMultipleOutput() +func NewMultipleOutputterFn(input []byte) multipleOutputterFn { + return multipleOutputterFn{ + input: input, + } +} + +type multipleOutputterFn struct { + input []byte +} + +func (o multipleOutputterFn) New() (Outputter, error) { + var r resources + err := json.Unmarshal(o.input, &r) + if err != nil { + return MultipleOutputter{}, err + } + + return MultipleOutputter{ + outputFn: multiplePlaintextOutputFn, + resources: r, + resourceJSON: o.input, + }, nil +} + +type MultipleOutputter struct { + outputFn PlaintextOutputFn + resources resources + resourceJSON []byte +} + +func (o MultipleOutputter) JSON() string { + return string(o.resourceJSON) +} + +func (o MultipleOutputter) String() string { + return formatColl(o.resources.Items, o.outputFn) +} diff --git a/internal/output/multiple_outputter_test.go b/internal/output/multiple_outputter_test.go new file mode 100644 index 00000000..b22f449e --- /dev/null +++ b/internal/output/multiple_outputter_test.go @@ -0,0 +1,59 @@ +package output_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "ldcli/internal/output" +) + +func TestMultipleOutputter_JSON(t *testing.T) { + input := []byte(`{ + "items": [ + { + "key": "test-key1", + "name": "test-name1", + "other": "another-value2" + }, + { + "key": "test-key2", + "name": "test-name2", + "other": "another-value2" + } + ] + }`) + output, err := output.CmdOutput( + "json", + output.NewMultipleOutputterFn(input), + ) + + require.NoError(t, err) + assert.JSONEq(t, output, string(input)) +} + +func TestMultipleOutputter_String(t *testing.T) { + input := []byte(`{ + "items": [ + { + "key": "test-key1", + "name": "test-name1", + "other": "another-value2" + }, + { + "key": "test-key2", + "name": "test-name2", + "other": "another-value2" + } + ] + }`) + expected := "* test-name1 (test-key1)\n* test-name2 (test-key2)" + output, err := output.CmdOutput( + "plaintext", + output.NewMultipleOutputterFn(input), + ) + + require.NoError(t, err) + assert.Equal(t, expected, output) +} diff --git a/internal/output/output.go b/internal/output/output.go new file mode 100644 index 00000000..eaea59e4 --- /dev/null +++ b/internal/output/output.go @@ -0,0 +1,67 @@ +package output + +import ( + "strings" + + "ldcli/internal/errors" +) + +var ErrInvalidOutputKind = errors.NewError("output is invalid") + +// Outputter defines the different ways a command's response can be formatted based on +// user input. +type Outputter interface { + JSON() string + String() string +} + +// OutputterFn is a factory to build the right outputter. By adding an layer of abstraction, +// it lets us push back the error handling from where a caller provides the input to where +// the caller builds the outputter. +type OutputterFn interface { + New() (Outputter, error) +} + +// PlaintextOutputFn represents the various ways to output a resource or resources. +type PlaintextOutputFn func(resource) string + +// resource is the subset of data we need to display a command's plain text response for a single +// resource. +type resource struct { + Key string `json:"key"` + Name string `json:"name"` +} + +// resources is the subset of data we need to display a command's plain text response for a list +// of resources. +type resources struct { + Items []resource `json:"items"` +} + +// CmdOutput returns a command's response as a string formatted based on the user's requested type. +func CmdOutput(outputKind string, outputter OutputterFn) (string, error) { + o, err := outputter.New() + if err != nil { + return "", err + } + + switch outputKind { + case "json": + return o.JSON(), nil + case "plaintext": + return o.String(), nil + } + + return "", ErrInvalidOutputKind +} + +// FormatColl applies a formatting function to every element in the collection and returns it as a +// string. +func formatColl[T any](coll []T, formatFn func(T) string) string { + lst := make([]string, 0, len(coll)) + for _, c := range coll { + lst = append(lst, formatFn(c)) + } + + return strings.Join(lst, "\n") +} diff --git a/internal/output/singular_outputter.go b/internal/output/singular_outputter.go new file mode 100644 index 00000000..4c953c55 --- /dev/null +++ b/internal/output/singular_outputter.go @@ -0,0 +1,49 @@ +package output + +import ( + "encoding/json" + "fmt" +) + +var singularPlaintextOutputFn = func(r resource) string { + return fmt.Sprintf("%s (%s)", r.Name, r.Key) +} + +// TODO: rename this to be "cleaner"? -- NewSingularOutput() +func NewSingularOutputterFn(input []byte) singularOutputterFn { + return singularOutputterFn{ + input: input, + } +} + +type singularOutputterFn struct { + input []byte +} + +func (o singularOutputterFn) New() (Outputter, error) { + var r resource + err := json.Unmarshal(o.input, &r) + if err != nil { + return SingularOutputter{}, err + } + + return SingularOutputter{ + outputFn: singularPlaintextOutputFn, + resource: r, + resourceJSON: o.input, + }, nil +} + +type SingularOutputter struct { + outputFn PlaintextOutputFn + resource resource + resourceJSON []byte +} + +func (o SingularOutputter) JSON() string { + return string(o.resourceJSON) +} + +func (o SingularOutputter) String() string { + return formatColl([]resource{o.resource}, o.outputFn) +} diff --git a/internal/output/singular_outputter_test.go b/internal/output/singular_outputter_test.go new file mode 100644 index 00000000..81be51a1 --- /dev/null +++ b/internal/output/singular_outputter_test.go @@ -0,0 +1,41 @@ +package output_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "ldcli/internal/output" +) + +func TestSingularOutputter_JSON(t *testing.T) { + input := []byte(`{ + "key": "test-key", + "name": "test-name", + "other": "another-value" + }`) + output, err := output.CmdOutput( + "json", + output.NewSingularOutputterFn(input), + ) + + require.NoError(t, err) + assert.JSONEq(t, output, string(input)) +} + +func TestSingularOutputter_String(t *testing.T) { + input := []byte(`{ + "key": "test-key", + "name": "test-name", + "other": "another-value" + }`) + expected := "test-name (test-key)" + output, err := output.CmdOutput( + "plaintext", + output.NewSingularOutputterFn(input), + ) + + require.NoError(t, err) + assert.Equal(t, expected, output) +}