From b7bb9d50b6cf42c557a2e713eceb56afad6b3448 Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Thu, 9 May 2024 13:40:58 -0700 Subject: [PATCH 1/3] WIP --- internal/output/output.go | 8 +++- internal/output/resource_output.go | 62 +++++++++++++++++-------- internal/output/resource_output_test.go | 30 +++++++++++- 3 files changed, 79 insertions(+), 21 deletions(-) diff --git a/internal/output/output.go b/internal/output/output.go index 04ee0717..b40fcd24 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -54,10 +54,16 @@ type PlaintextOutputFn func(resource) string // We're trading off type safety for easy of use instead of defining a type for each expected resource. type resource map[string]interface{} +type link map[string]string + +type links map[string]link + // 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"` + Items []resource `json:"items"` + Links links `json:"_links"` + TotalCount int `json:"totalCount"` } // CmdOutputSingular builds a command response based on the flag the user provided and the shape of diff --git a/internal/output/resource_output.go b/internal/output/resource_output.go index 2cfbb6da..0c28e451 100644 --- a/internal/output/resource_output.go +++ b/internal/output/resource_output.go @@ -3,6 +3,8 @@ package output import ( "encoding/json" "fmt" + "net/url" + "strconv" "strings" "github.com/pkg/errors" @@ -45,30 +47,52 @@ func CmdOutput(action string, outputKind string, input []byte) (string, error) { // no success message } - if isMultipleResponse { - if len(maybeResources.Items) == 0 { - return "No items found", nil - } + if !isMultipleResponse { + return plaintextOutput(SingularPlaintextOutputFn(maybeResource), successMessage), nil + } - // the response could have various properties we want to show - keyExists := func(key string) bool { _, ok := maybeResources.Items[0][key]; return ok } - outputFn := MultiplePlaintextOutputFn - switch { - case keyExists("email"): - outputFn = MultipleEmailPlaintextOutputFn - case keyExists("_id"): - outputFn = MultipleIDPlaintextOutputFn - } + if len(maybeResources.Items) == 0 { + return "No items found", nil + } - items := make([]string, 0, len(maybeResources.Items)) - for _, i := range maybeResources.Items { - items = append(items, outputFn(i)) - } + // the response could have various properties we want to show + keyExists := func(key string) bool { _, ok := maybeResources.Items[0][key]; return ok } + outputFn := MultiplePlaintextOutputFn + switch { + case keyExists("email"): + outputFn = MultipleEmailPlaintextOutputFn + case keyExists("_id"): + outputFn = MultipleIDPlaintextOutputFn + } - return plaintextOutput("\n"+strings.Join(items, "\n"), successMessage), nil + items := make([]string, 0, len(maybeResources.Items)) + for _, i := range maybeResources.Items { + items = append(items, outputFn(i)) } - return plaintextOutput(SingularPlaintextOutputFn(maybeResource), successMessage), nil + // fmt.Println(">>> checking", maybeResources.TotalCount) + // spew.Dump(maybeResources.Links) + var ( + pagination string + limit int + offset int + ) + if self, ok := maybeResources.Links["self"]; ok { + selfURL, _ := url.Parse(self["href"]) + limit, _ = strconv.Atoi(selfURL.Query().Get("limit")) + offset, _ = strconv.Atoi(selfURL.Query().Get("offset")) + fmt.Println(">>> found", limit, offset) + } + + pagination = fmt.Sprintf( + "Showing results %d - %d of %d. Use --offset %d for additional results.", + offset+1, + offset+limit, + maybeResources.TotalCount, + offset+limit, + ) + + return plaintextOutput("\n"+strings.Join(items, "\n"), successMessage) + "\n" + pagination, nil } func plaintextOutput(out string, successMessage string) string { diff --git a/internal/output/resource_output_test.go b/internal/output/resource_output_test.go index ae1639a8..cde76c47 100644 --- a/internal/output/resource_output_test.go +++ b/internal/output/resource_output_test.go @@ -12,6 +12,34 @@ import ( ) func TestCmdOutput(t *testing.T) { + t.Run("with paginated multiple resources", func(t *testing.T) { + input := `{ + "_links": { + "self": { + "href": "/my-resources?limit=5&offset=5", + "type": "application/json" + } + }, + "items": [ + { + "key": "test-key", + "name": "test-name" + } + ], + "totalCount": 100 + }` + + t.Run("shows pagination", func(t *testing.T) { + expected := "\n* test-name (test-key)" + expected += "\nShowing results 6 - 10 of 100. Use --offset 10 for additional results." + + result, err := output.CmdOutput("list", "plaintext", []byte(input)) + + require.NoError(t, err) + assert.Equal(t, expected, result) + }) + }) + t.Run("with multiple resources with an ID and name", func(t *testing.T) { input := `{ "items": [ @@ -23,7 +51,7 @@ func TestCmdOutput(t *testing.T) { }` t.Run("with plaintext output", func(t *testing.T) { - t.Run("returns a success message", func(t *testing.T) { + t.Run("returns a list of resources", func(t *testing.T) { expected := "\n* test-name (test-id)" result, err := output.CmdOutput("list", "plaintext", []byte(input)) From e947fc651b3af6f112ed9edc94c48396c3eff418 Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Thu, 9 May 2024 15:46:47 -0700 Subject: [PATCH 2/3] Show pagination --- internal/output/resource_output.go | 34 +++++++------ internal/output/resource_output_test.go | 65 +++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/internal/output/resource_output.go b/internal/output/resource_output.go index 0c28e451..e801be01 100644 --- a/internal/output/resource_output.go +++ b/internal/output/resource_output.go @@ -3,6 +3,7 @@ package output import ( "encoding/json" "fmt" + "math" "net/url" "strconv" "strings" @@ -48,7 +49,7 @@ func CmdOutput(action string, outputKind string, input []byte) (string, error) { } if !isMultipleResponse { - return plaintextOutput(SingularPlaintextOutputFn(maybeResource), successMessage), nil + return plaintextOutput(SingularPlaintextOutputFn(maybeResource), successMessage+" "), nil } if len(maybeResources.Items) == 0 { @@ -70,34 +71,37 @@ func CmdOutput(action string, outputKind string, input []byte) (string, error) { items = append(items, outputFn(i)) } - // fmt.Println(">>> checking", maybeResources.TotalCount) - // spew.Dump(maybeResources.Links) var ( pagination string limit int offset int ) - if self, ok := maybeResources.Links["self"]; ok { + self, ok := maybeResources.Links["self"] + if ok && maybeResources.TotalCount > 0 { selfURL, _ := url.Parse(self["href"]) limit, _ = strconv.Atoi(selfURL.Query().Get("limit")) offset, _ = strconv.Atoi(selfURL.Query().Get("offset")) - fmt.Println(">>> found", limit, offset) + maxResults := int(math.Min(float64(offset+limit), float64(maybeResources.TotalCount))) + pagination = fmt.Sprintf( + "\nShowing results %d - %d of %d.", + offset+1, + maxResults, + maybeResources.TotalCount, + ) + if offset+limit < maybeResources.TotalCount { + pagination += fmt.Sprintf(" Use --offset %d for additional results.", offset+limit) + } } - pagination = fmt.Sprintf( - "Showing results %d - %d of %d. Use --offset %d for additional results.", - offset+1, - offset+limit, - maybeResources.TotalCount, - offset+limit, - ) - - return plaintextOutput("\n"+strings.Join(items, "\n"), successMessage) + "\n" + pagination, nil + if successMessage != "" { + successMessage += "\n" + } + return plaintextOutput(strings.Join(items, "\n"), successMessage) + pagination, nil } func plaintextOutput(out string, successMessage string) string { if successMessage != "" { - return fmt.Sprintf("%s %s", successMessage, out) + return fmt.Sprintf("%s%s", successMessage, out) } return out diff --git a/internal/output/resource_output_test.go b/internal/output/resource_output_test.go index cde76c47..46f76c92 100644 --- a/internal/output/resource_output_test.go +++ b/internal/output/resource_output_test.go @@ -12,7 +12,36 @@ import ( ) func TestCmdOutput(t *testing.T) { + // with no additional pagination - does not show offset help t.Run("with paginated multiple resources", func(t *testing.T) { + input := `{ + "_links": { + "self": { + "href": "/my-resources?limit=5", + "type": "application/json" + } + }, + "items": [ + { + "key": "test-key", + "name": "test-name" + } + ], + "totalCount": 100 + }` + + t.Run("shows pagination", func(t *testing.T) { + expected := "* test-name (test-key)" + expected += "\nShowing results 1 - 5 of 100. Use --offset 5 for additional results." + + result, err := output.CmdOutput("list", "plaintext", []byte(input)) + + require.NoError(t, err) + assert.Equal(t, expected, result) + }) + }) + + t.Run("with a paginated offset shows pagination", func(t *testing.T) { input := `{ "_links": { "self": { @@ -30,7 +59,7 @@ func TestCmdOutput(t *testing.T) { }` t.Run("shows pagination", func(t *testing.T) { - expected := "\n* test-name (test-key)" + expected := "* test-name (test-key)" expected += "\nShowing results 6 - 10 of 100. Use --offset 10 for additional results." result, err := output.CmdOutput("list", "plaintext", []byte(input)) @@ -40,6 +69,34 @@ func TestCmdOutput(t *testing.T) { }) }) + t.Run("with no additional pagination does not show offset help", func(t *testing.T) { + input := `{ + "_links": { + "self": { + "href": "/my-resources?limit=5&offset=95", + "type": "application/json" + } + }, + "items": [ + { + "key": "test-key", + "name": "test-name" + } + ], + "totalCount": 100 + }` + + t.Run("shows pagination", func(t *testing.T) { + expected := "* test-name (test-key)" + expected += "\nShowing results 96 - 100 of 100." + + result, err := output.CmdOutput("list", "plaintext", []byte(input)) + + require.NoError(t, err) + assert.Equal(t, expected, result) + }) + }) + t.Run("with multiple resources with an ID and name", func(t *testing.T) { input := `{ "items": [ @@ -52,7 +109,7 @@ func TestCmdOutput(t *testing.T) { t.Run("with plaintext output", func(t *testing.T) { t.Run("returns a list of resources", func(t *testing.T) { - expected := "\n* test-name (test-id)" + expected := "* test-name (test-id)" result, err := output.CmdOutput("list", "plaintext", []byte(input)) @@ -112,7 +169,7 @@ func TestCmdOutput(t *testing.T) { t.Run("with plaintext output", func(t *testing.T) { t.Run("returns a success message", func(t *testing.T) { - expected := "Successfully created \n* test-name (test-key)" + expected := "Successfully created\n* test-name (test-key)" result, err := output.CmdOutput("create", "plaintext", []byte(input)) @@ -144,7 +201,7 @@ func TestCmdOutput(t *testing.T) { t.Run("with plaintext output", func(t *testing.T) { t.Run("returns a success message", func(t *testing.T) { - expected := "Successfully created \n* test-email (test-id)" + expected := "Successfully created\n* test-email (test-id)" result, err := output.CmdOutput("create", "plaintext", []byte(input)) From 670d99bafe90e0c6a26577d80cd91bac8031a9c4 Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Thu, 9 May 2024 16:10:51 -0700 Subject: [PATCH 3/3] Refactor to table tests --- internal/output/resource_output_test.go | 122 +++++++++--------------- 1 file changed, 45 insertions(+), 77 deletions(-) diff --git a/internal/output/resource_output_test.go b/internal/output/resource_output_test.go index 46f76c92..4d1ff9dd 100644 --- a/internal/output/resource_output_test.go +++ b/internal/output/resource_output_test.go @@ -2,6 +2,7 @@ package output_test import ( "encoding/json" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -12,89 +13,56 @@ import ( ) func TestCmdOutput(t *testing.T) { - // with no additional pagination - does not show offset help t.Run("with paginated multiple resources", func(t *testing.T) { - input := `{ - "_links": { - "self": { - "href": "/my-resources?limit=5", - "type": "application/json" - } + tests := map[string]struct { + limit int + offset int + expected string + }{ + "shows pagination": { + limit: 5, + expected: "* test-name (test-key)\nShowing results 1 - 5 of 100. Use --offset 5 for additional results.", }, - "items": [ - { - "key": "test-key", - "name": "test-name" - } - ], - "totalCount": 100 - }` - - t.Run("shows pagination", func(t *testing.T) { - expected := "* test-name (test-key)" - expected += "\nShowing results 1 - 5 of 100. Use --offset 5 for additional results." - - result, err := output.CmdOutput("list", "plaintext", []byte(input)) - - require.NoError(t, err) - assert.Equal(t, expected, result) - }) - }) - - t.Run("with a paginated offset shows pagination", func(t *testing.T) { - input := `{ - "_links": { - "self": { - "href": "/my-resources?limit=5&offset=5", - "type": "application/json" - } + "with a paginated offset shows pagination": { + limit: 5, + offset: 5, + expected: "* test-name (test-key)\nShowing results 6 - 10 of 100. Use --offset 10 for additional results.", }, - "items": [ - { - "key": "test-key", - "name": "test-name" - } - ], - "totalCount": 100 - }` - - t.Run("shows pagination", func(t *testing.T) { - expected := "* test-name (test-key)" - expected += "\nShowing results 6 - 10 of 100. Use --offset 10 for additional results." - - result, err := output.CmdOutput("list", "plaintext", []byte(input)) - - require.NoError(t, err) - assert.Equal(t, expected, result) - }) - }) - - t.Run("with no additional pagination does not show offset help", func(t *testing.T) { - input := `{ - "_links": { - "self": { - "href": "/my-resources?limit=5&offset=95", - "type": "application/json" - } + "with no additional pagination does not show offset help": { + limit: 5, + offset: 95, + expected: "* test-name (test-key)\nShowing results 96 - 100 of 100.", }, - "items": [ - { - "key": "test-key", - "name": "test-name" - } - ], - "totalCount": 100 - }` - - t.Run("shows pagination", func(t *testing.T) { - expected := "* test-name (test-key)" - expected += "\nShowing results 96 - 100 of 100." + } + for name, tt := range tests { + tt := tt + t.Run(name, func(t *testing.T) { + input := fmt.Sprintf( + `{ + "_links": { + "self": { + "href": "/my-resources?limit=%d&offset=%d", + "type": "application/json" + } + }, + "items": [ + { + "key": "test-key", + "name": "test-name" + } + ], + "totalCount": 100 + }`, + tt.limit, + tt.offset, + ) - result, err := output.CmdOutput("list", "plaintext", []byte(input)) + result, err := output.CmdOutput("list", "plaintext", []byte(input)) - require.NoError(t, err) - assert.Equal(t, expected, result) - }) + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } }) t.Run("with multiple resources with an ID and name", func(t *testing.T) {