From 7932c13c0aa561b2393dff84090c21b9be0de063 Mon Sep 17 00:00:00 2001 From: Mark Benjamin Date: Fri, 6 Dec 2024 13:53:50 -0500 Subject: [PATCH] Improve time fields (#171) * Add cobra time arg type * Add time parsing to form and structs * Add placeholder and suggestions * Add missing enum tests --- cmd/deploylist.go | 9 ++--- cmd/joblist.go | 13 ++++--- cmd/logs.go | 9 +++-- pkg/command/form.go | 35 ++++++++++++++++- pkg/command/form_test.go | 53 +++++++++++++++++++++++++ pkg/command/inputs.go | 25 ++++++++++++ pkg/command/inputs_test.go | 19 +++++++++ pkg/command/time.go | 80 +++++++++++++++++++++++++++++++++++--- pkg/command/time_test.go | 62 ++++++++++++++++++++++++++--- pkg/tui/views/logview.go | 23 +++++------ 10 files changed, 291 insertions(+), 37 deletions(-) diff --git a/cmd/deploylist.go b/cmd/deploylist.go index 42c2e76..821ad82 100644 --- a/cmd/deploylist.go +++ b/cmd/deploylist.go @@ -11,7 +11,6 @@ import ( "github.com/renderinc/cli/pkg/command" "github.com/renderinc/cli/pkg/dashboard" "github.com/renderinc/cli/pkg/deploy" - "github.com/renderinc/cli/pkg/pointers" "github.com/renderinc/cli/pkg/resource" "github.com/renderinc/cli/pkg/text" "github.com/renderinc/cli/pkg/tui/views" @@ -57,14 +56,14 @@ func interactiveDeployList(cmd *cobra.Command, input views.DeployListInput) tea. } func commandsForDeploy(dep *client.Deploy, serviceID, serviceType string) []views.PaletteCommand { - var startTime *string + var startTime *command.TimeOrRelative if dep.CreatedAt != nil { - startTime = pointers.From(dep.CreatedAt.String()) + startTime = &command.TimeOrRelative{T: dep.CreatedAt} } - var endTime *string + var endTime *command.TimeOrRelative if dep.FinishedAt != nil { - endTime = pointers.From(dep.FinishedAt.String()) + endTime = &command.TimeOrRelative{T: dep.FinishedAt} } commands := []views.PaletteCommand{ diff --git a/cmd/joblist.go b/cmd/joblist.go index ff44cb9..1abf572 100644 --- a/cmd/joblist.go +++ b/cmd/joblist.go @@ -11,7 +11,6 @@ import ( clientjob "github.com/renderinc/cli/pkg/client/jobs" "github.com/renderinc/cli/pkg/command" "github.com/renderinc/cli/pkg/job" - "github.com/renderinc/cli/pkg/pointers" "github.com/renderinc/cli/pkg/resource" "github.com/renderinc/cli/pkg/text" "github.com/renderinc/cli/pkg/tui/views" @@ -41,7 +40,9 @@ func interactiveJobList(cmd *cobra.Command, input views.JobListInput) tea.Cmd { "Jobs", &input, views.NewServiceList(ctx, views.ServiceInput{ - Types: []client.ServiceType{client.WebService, client.BackgroundWorker, client.PrivateService, client.CronJob}, + Types: []client.ServiceType{ + client.WebService, client.BackgroundWorker, client.PrivateService, client.CronJob, + }, }, func(ctx context.Context, r resource.Resource) tea.Cmd { input.ServiceID = r.ID() return InteractiveJobList(ctx, input, resource.BreadcrumbForResource(r)) @@ -58,14 +59,14 @@ func interactiveJobList(cmd *cobra.Command, input views.JobListInput) tea.Cmd { } func commandsForJob(j *clientjob.Job) []views.PaletteCommand { - var startTime *string + var startTime *command.TimeOrRelative if j.StartedAt != nil { - startTime = pointers.From(j.StartedAt.String()) + startTime = &command.TimeOrRelative{T: j.StartedAt} } - var endTime *string + var endTime *command.TimeOrRelative if j.FinishedAt != nil { - endTime = pointers.From(j.FinishedAt.String()) + endTime = &command.TimeOrRelative{T: j.FinishedAt} } commands := []views.PaletteCommand{ diff --git a/cmd/logs.go b/cmd/logs.go index b81ac3f..15163bc 100644 --- a/cmd/logs.go +++ b/cmd/logs.go @@ -88,7 +88,7 @@ func TailResourceLogs(ctx context.Context, resourceID string) tea.Cmd { return InteractiveLogs( ctx, views.LogInput{ - StartTime: pointers.From(time.Now().Format(time.RFC3339)), + StartTime: &command.TimeOrRelative{T: pointers.From(time.Now())}, ResourceIDs: []string{resourceID}, Tail: true, }, "Logs") @@ -128,6 +128,9 @@ func init() { "GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS", "HEAD", "CONNECT", "TRACE", }, true) + startTimeFlag := command.NewTimeInput() + endTimeFlag := command.NewTimeInput() + LogsCmd.RunE = func(cmd *cobra.Command, args []string) error { var input views.LogInput err := command.ParseCommand(cmd, args, &input) @@ -156,8 +159,8 @@ func init() { rootCmd.AddCommand(LogsCmd) LogsCmd.Flags().StringSliceP("resources", "r", []string{}, "A list of comma separated resource IDs to query. Required in non-interactive mode.") - LogsCmd.Flags().String("start", "", "The start time of the logs to query") - LogsCmd.Flags().String("end", "", "The end time of the logs to query") + LogsCmd.Flags().Var(startTimeFlag, "start", "The start time of the logs to query") + LogsCmd.Flags().Var(endTimeFlag, "end", "The end time of the logs to query") LogsCmd.Flags().StringSlice("text", []string{}, "A list of comma separated strings to search for in the logs") LogsCmd.Flags().Var(levelFlag, "level", "A list of comma separated log levels to query") LogsCmd.Flags().Var(logTypeFlag, "type", "A list of comma separated log types to query") diff --git a/pkg/command/form.go b/pkg/command/form.go index 64f692b..fbd1063 100644 --- a/pkg/command/form.go +++ b/pkg/command/form.go @@ -5,6 +5,7 @@ import ( "reflect" "strconv" "strings" + "time" "github.com/charmbracelet/huh" "github.com/charmbracelet/x/ansi" @@ -63,7 +64,13 @@ func FormValuesFromStruct(v any) FormValues { case reflect.Ptr: if elemField.IsNil() { formValues[cliTag] = NewStringFormValue("") - break + continue + } + + if field.Type == reflect.TypeOf(&TimeOrRelative{}) { + val := elemField.Interface().(*TimeOrRelative) + formValues[cliTag] = NewStringFormValue(val.String()) + continue } switch field.Type.Elem().Kind() { @@ -159,6 +166,21 @@ func StructFromFormValues(formValues FormValues, v any) error { switch field.Type.Kind() { case reflect.Ptr: + if field.Type == reflect.TypeOf(&TimeOrRelative{}) { + val, ok := formValues[cliTag] + if !ok { + continue + } + + timeOrRelative, err := ParseTime(time.Now(), pointers.From(val.String())) + if err != nil { + return err + } + + elemField.Set(reflect.ValueOf(timeOrRelative)) + continue + } + switch field.Type.Elem().Kind() { case reflect.String: val, ok := formValues[cliTag] @@ -337,6 +359,17 @@ func HuhFormFields(cmd *cobra.Command, v any) ([]huh.Field, FormValues) { huhFieldMap[flag.Name] = huh.NewSelect[string]().Key(flag.Name).Title(flag.Name).Description(wrappedDescription).Options(options...).Value((*string)(strValue)) } + } else if flag.Value.Type() == TimeType { + timeValue := NewStringFormValue(value.String()) + formValues[flag.Name] = timeValue + + huhFieldMap[flag.Name] = huh.NewInput(). + Key(flag.Name). + Title(flag.Name). + Description(wrappedDescription). + Value((*string)(timeValue)). + Placeholder(fmt.Sprintf("Relative time or %s", time.RFC3339)). + SuggestionsFunc(func() []string { return TimeSuggestion(timeValue.String()) }, timeValue) } else { strValue := NewStringFormValue(value.String()) formValues[flag.Name] = strValue diff --git a/pkg/command/form_test.go b/pkg/command/form_test.go index 413c179..d56bc0f 100644 --- a/pkg/command/form_test.go +++ b/pkg/command/form_test.go @@ -2,6 +2,7 @@ package command_test import ( "testing" + "time" "github.com/charmbracelet/huh" "github.com/renderinc/cli/pkg/command" @@ -72,6 +73,38 @@ func TestStructFromFormValues(t *testing.T) { require.NoError(t, command.StructFromFormValues(formValues, &v)) require.Equal(t, []string{"owner-id-1", "owner-id-2"}, v.OwnerIDs) }) + + t.Run("converts time type", func(t *testing.T) { + type testStruct struct { + Time *command.TimeOrRelative `cli:"time"` + } + str := "1m" + formValues := command.FormValues{"time": command.NewStringFormValue(str)} + v := testStruct{} + require.NoError(t, command.StructFromFormValues(formValues, &v)) + require.Equal(t, "1m", v.Time.String()) + require.WithinDuration(t, *v.Time.T, time.Now().Add(-time.Minute), time.Second) + }) + + t.Run("converts enum type", func(t *testing.T) { + type testStruct struct { + Foo string `cli:"foo"` + } + formValues := command.FormValues{"foo": command.NewStringFormValue("value")} + v := testStruct{} + require.NoError(t, command.StructFromFormValues(formValues, &v)) + require.Equal(t, "value", v.Foo) + }) + + t.Run("converts enum multi type", func(t *testing.T) { + type testStruct struct { + Foo []string `cli:"foo"` + } + formValues := command.FormValues{"foo": command.NewStringFormValue("value,other")} + v := testStruct{} + require.NoError(t, command.StructFromFormValues(formValues, &v)) + require.Equal(t, []string{"value", "other"}, v.Foo) + }) } func TestHuhForm(t *testing.T) { @@ -116,4 +149,24 @@ func TestHuhForm(t *testing.T) { require.Contains(t, form.View(), "multi choice 3") require.Contains(t, form.View(), "single choice 2") }) + + t.Run("creates form with time", func(t *testing.T) { + type testStruct struct { + Foo *command.TimeOrRelative `cli:"foo"` + } + v := testStruct{} + cmd := cobra.Command{} + + // foo is multi select + fooInput := command.NewTimeInput() + cmd.Flags().Var(fooInput, "foo", "") + + fields, _ := command.HuhFormFields(&cmd, &v) + form := huh.NewForm(huh.NewGroup(fields...)) + form.Init() + + require.Contains(t, form.View(), "foo") + // Find placeholder text + require.Contains(t, form.View(), "Relative time or") + }) } diff --git a/pkg/command/inputs.go b/pkg/command/inputs.go index 17a7a6e..0cd6873 100644 --- a/pkg/command/inputs.go +++ b/pkg/command/inputs.go @@ -34,6 +34,21 @@ func getArgValue(tag string, args []string) (*string, error) { return &args[index], nil } +func getTimeValue(flags *pflag.FlagSet, tag string) (*TimeOrRelative, error) { + if flag := flags.Lookup(tag); flag != nil { + if flag.Value.Type() == TimeType { + cobraTime, ok := flag.Value.(*CobraTime) + if !ok { + return nil, fmt.Errorf("unexpected time type") + } + val := cobraTime.t + return val, nil + } + } + + return nil, nil +} + func getStringValue(flags *pflag.FlagSet, args []string, tag string) (*string, error) { if isArg(tag) { if val, err := getArgValue(tag, args); err != nil { @@ -176,6 +191,16 @@ func ParseCommand(cmd *cobra.Command, args []string, v any) error { switch field.Type.Kind() { case reflect.Ptr: + if field.Type == reflect.TypeOf(&TimeOrRelative{}) { + val, err := getTimeValue(flags, cliTag) + if err != nil { + return err + } + + elemField.Set(reflect.ValueOf(val)) + continue + } + switch field.Type.Elem().Kind() { case reflect.String: val, err := getStringValue(flags, args, cliTag) diff --git a/pkg/command/inputs_test.go b/pkg/command/inputs_test.go index aad4aad..f7673ed 100644 --- a/pkg/command/inputs_test.go +++ b/pkg/command/inputs_test.go @@ -2,6 +2,7 @@ package command_test import ( "testing" + "time" "github.com/renderinc/cli/pkg/command" "github.com/renderinc/cli/pkg/pointers" @@ -59,6 +60,24 @@ func TestParseCommand(t *testing.T) { require.Equal(t, []string{"a", "c"}, v.Foo) }) + t.Run("parse time", func(t *testing.T) { + timeInput := command.NewTimeInput() + + type testStruct struct { + Foo *command.TimeOrRelative `cli:"foo"` + } + var v testStruct + cmd := &cobra.Command{} + cmd.Flags().Var(timeInput, "foo", "") + require.NoError(t, cmd.ParseFlags([]string{"--foo", "5m"})) + + err := command.ParseCommand(cmd, []string{}, &v) + require.NoError(t, err) + + require.Equal(t, "5m", *v.Foo.Relative) + require.WithinDuration(t, *v.Foo.T, time.Now().Add(-5*time.Minute), time.Second) + }) + t.Run("parse pointer", func(t *testing.T) { type testStruct struct { Foo *string `cli:"foo"` diff --git a/pkg/command/time.go b/pkg/command/time.go index 424443d..2247c63 100644 --- a/pkg/command/time.go +++ b/pkg/command/time.go @@ -4,9 +4,39 @@ import ( "fmt" "regexp" "strconv" + "strings" "time" ) +var RFC3339RegexString = []string{ + `\d`, `\d`, `\d`, `\d`, `-`, `\d`, `\d`, `-`, `\d`, `\d`, + `T`, `\d`, `\d`, `:`, `\d`, `\d`, `:`, `\d`, `\d`, + `Z`, `\d`, `\d`, `:`, `\d`, `\d`, +} + +func TimeSuggestion(str string) []string { + var suggestion string + if i, err := strconv.Atoi(str); err == nil && i <= 60 { + suggestion = fmt.Sprintf("%dm", i) + } else if re, err := regexp.Compile(strings.Join(RFC3339RegexString[:len(str)], "")); err == nil && re.MatchString(str) { + suggestion = str + time.RFC3339[len(str):] + } + + return []string{suggestion} +} + +type TimeOrRelative struct { + T *time.Time + Relative *string +} + +func (t *TimeOrRelative) String() string { + if t.Relative != nil { + return *t.Relative + } + return t.T.Format(time.RFC3339) +} + var relativeRegex = regexp.MustCompile(`^(\d+)([smhd])$`) var characterToDuration = map[string]time.Duration{ @@ -16,7 +46,7 @@ var characterToDuration = map[string]time.Duration{ "d": time.Hour * 24, } -func parseRelativeTime(now time.Time, str string) *time.Time { +func parseRelativeTime(now time.Time, str string) *TimeOrRelative { matches := relativeRegex.FindStringSubmatch(str) if len(matches) != 3 { return nil @@ -28,10 +58,10 @@ func parseRelativeTime(now time.Time, str string) *time.Time { } t := now.Add(-characterToDuration[matches[2]] * time.Duration(num)) - return &t + return &TimeOrRelative{T: &t, Relative: &str} } -func ParseTime(now time.Time, str *string) (*time.Time, error) { +func ParseTime(now time.Time, str *string) (*TimeOrRelative, error) { if str == nil || *str == "" { return nil, nil } @@ -40,10 +70,48 @@ func ParseTime(now time.Time, str *string) (*time.Time, error) { return t, nil } - t, err := time.Parse(time.RFC3339, *str) + absoluteTime, err := time.Parse(time.RFC3339, *str) + if err != nil { + return nil, fmt.Errorf("invalid timestamp, time must either be relative (1m, 5h, etc) or in RFC3339 format: %s", time.RFC3339) + } + + return &TimeOrRelative{T: &absoluteTime}, nil +} + +const ( + TimeType = "time" +) + +type CobraTime struct { + t *TimeOrRelative +} + +func NewTimeInput() *CobraTime { + return &CobraTime{} +} + +func (e *CobraTime) String() string { + if e.t == nil { + return "" + } + + return e.t.String() +} + +func (e *CobraTime) Set(v string) error { + t, err := ParseTime(time.Now(), &v) if err != nil { - return nil, fmt.Errorf("invalid timestamp: %w", err) + return err } - return &t, nil + e.t = t + return nil +} + +func (e *CobraTime) Type() string { + return TimeType +} + +func (e *CobraTime) Get() *TimeOrRelative { + return e.t } diff --git a/pkg/command/time_test.go b/pkg/command/time_test.go index a491e7d..b402243 100644 --- a/pkg/command/time_test.go +++ b/pkg/command/time_test.go @@ -15,27 +15,27 @@ func TestParseTime(t *testing.T) { tcs := []struct { name string str string - expected time.Time + expected command.TimeOrRelative }{ { name: "parse relative time minute", str: "1m", - expected: now.Add(-time.Minute), + expected: command.TimeOrRelative{T: pointers.From(now.Add(-time.Minute)), Relative: pointers.From("1m")}, }, { name: "parse relative time hour", str: "1h", - expected: now.Add(-time.Hour), + expected: command.TimeOrRelative{T: pointers.From(now.Add(-time.Hour)), Relative: pointers.From("1h")}, }, { name: "parse relative time day", str: "1d", - expected: now.Add(-24 * time.Hour), + expected: command.TimeOrRelative{T: pointers.From(now.Add(-24 * time.Hour)), Relative: pointers.From("1d")}, }, { name: "parse absolute time", str: now.Format(time.RFC3339), - expected: now, + expected: command.TimeOrRelative{T: &now}, }, } @@ -64,3 +64,55 @@ func TestParseTime(t *testing.T) { require.Error(t, err) }) } + +func TestCobraTime(t *testing.T) { + t.Run("set to relative time", func(t *testing.T) { + cobraTime := command.CobraTime{} + err := cobraTime.Set("1m") + require.NoError(t, err) + + require.Equal(t, "1m", cobraTime.String()) + require.WithinDuration(t, time.Now().Add(-time.Minute), *cobraTime.Get().T, time.Second) + }) + + t.Run("set to absolute time", func(t *testing.T) { + now := time.Now().Truncate(time.Second).UTC() + cobraTime := command.CobraTime{} + err := cobraTime.Set(now.Format(time.RFC3339)) + require.NoError(t, err) + + require.Equal(t, now.Format(time.RFC3339), cobraTime.String()) + require.Equal(t, now, *cobraTime.Get().T) + }) +} + +func TestTimeSuggestion(t *testing.T) { + tcs := []struct { + name string + str string + expected []string + }{ + { + name: "empty string", + str: "", + expected: []string{"2006-01-02T15:04:05Z07:00"}, + }, + { + name: "< 60 int", + str: "20", + expected: []string{"20m"}, + }, + { + name: "match time format", + str: "202", + expected: []string{"2026-01-02T15:04:05Z07:00"}, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + actual := command.TimeSuggestion(tc.str) + require.ElementsMatch(t, tc.expected, actual) + }) + } +} diff --git a/pkg/tui/views/logview.go b/pkg/tui/views/logview.go index d020627..6968aa6 100644 --- a/pkg/tui/views/logview.go +++ b/pkg/tui/views/logview.go @@ -47,8 +47,8 @@ type LogInput struct { Level []string `cli:"level"` Type []string `cli:"type"` - StartTime *string `cli:"start"` - EndTime *string `cli:"end"` + StartTime *command.TimeOrRelative `cli:"start"` + EndTime *command.TimeOrRelative `cli:"end"` Host []string `cli:"host"` StatusCode []string `cli:"status-code"` @@ -63,7 +63,6 @@ type LogInput struct { } func (l LogInput) ToParam() (*client.ListLogsParams, error) { - now := time.Now() ownerID, err := config.WorkspaceID() if err != nil { return nil, fmt.Errorf("error getting workspace ID: %v", err) @@ -73,21 +72,23 @@ func (l LogInput) ToParam() (*client.ListLogsParams, error) { l.Limit = 100 } - start, err := command.ParseTime(now, l.StartTime) - if err != nil { - return nil, err + var startTime *time.Time + if l.StartTime != nil { + startTime = l.StartTime.T } - end, err := command.ParseTime(now, l.EndTime) - if err != nil { - return nil, err + + var endTime *time.Time + if l.EndTime != nil { + endTime = l.EndTime.T } + return &client.ListLogsParams{ Resource: l.ResourceIDs, OwnerId: ownerID, Instance: pointers.FromArray(l.Instance), Limit: pointers.From(l.Limit), - StartTime: start, - EndTime: end, + StartTime: startTime, + EndTime: endTime, Text: pointers.FromArray(l.Text), Level: pointers.FromArray(l.Level), Type: pointers.FromArray(l.Type),