From 3338abc59d295bbc4442dc9c1da5f2e8872d170e Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Wed, 24 Jun 2026 14:16:40 -0700 Subject: [PATCH] Normalize account and du commands to shared JSON operation output Extends jsonOperationOutput normalization to account and du commands. Both now output the consistent input/results/warnings structure. Adds completion command registration for shell completions. --- README.md | 45 +++++++---- cmd/account.go | 29 ++++--- cmd/account_test.go | 45 +++++++++-- cmd/completion.go | 186 ++++++++++++++++++++++++++++++++++++++++++++ cmd/du.go | 13 +++- cmd/du_test.go | 73 ++++++++++++----- cmd/root_test.go | 36 +++++++++ 7 files changed, 373 insertions(+), 54 deletions(-) create mode 100644 cmd/completion.go diff --git a/README.md b/README.md index ab6cdfc..f6eb94d 100644 --- a/README.md +++ b/README.md @@ -158,7 +158,7 @@ Structured success output is rolling out command by command. Currently migrated Command results and JSON errors are written to stdout. Status, progress, human-facing warnings, diagnostics, and verbose logs are written to stderr. JSON errors include a `warnings` array for machine-actionable warnings; it is `[]` when no warnings are present. New operation-style JSON payloads should use the same `warnings` field. -Successful JSON responses are command-specific. Operation commands return an `input` object, a `results` array, and a `warnings` array. For commands such as `mkdir`, each result reports what happened to the requested path: +Successful JSON responses are command-specific. Operation-style commands return an `input` object, a `results` array, and a `warnings` array. For commands such as `mkdir`, each result reports what happened to the requested path: ```json { @@ -299,28 +299,45 @@ Commands that return entry lists, such as `ls`, `search`, and `revs`, return an } ``` -Account and usage commands return command-specific objects: +Account and usage commands use the operation-style wrapper with a single result: ```json { "input": {}, - "account": { - "type": "full", - "account_id": "dbid:...", - "email": "user@example.com", - "email_verified": true, - "disabled": false - } + "results": [ + { + "kind": "account", + "input": {}, + "result": { + "type": "full", + "account_id": "dbid:...", + "email": "user@example.com", + "email_verified": true, + "disabled": false + } + } + ], + "warnings": [] } ``` ```json { - "used": 123, - "allocation": { - "type": "individual", - "allocated": 100000 - } + "input": {}, + "results": [ + { + "kind": "space_usage", + "input": {}, + "result": { + "used": 123, + "allocation": { + "type": "individual", + "allocated": 100000 + } + } + } + ], + "warnings": [] } ``` diff --git a/cmd/account.go b/cmd/account.go index dedaa8b..71c919f 100644 --- a/cmd/account.go +++ b/cmd/account.go @@ -28,11 +28,6 @@ type accountInput struct { AccountID string `json:"account_id,omitempty"` } -type accountOutput struct { - Input accountInput `json:"input"` - Account jsonAccount `json:"account"` -} - type jsonAccount struct { Type string `json:"type"` AccountID string `json:"account_id"` @@ -64,6 +59,8 @@ type jsonAccountTeam struct { MemberID string `json:"member_id,omitempty"` } +const accountKindAccount = "account" + // renderFullAccount prints the account details returned by GetCurrentAccount. func renderFullAccount(out io.Writer, fa *users.FullAccount) error { w := new(tabwriter.Writer) @@ -117,12 +114,10 @@ func account(cmd *cobra.Command, args []string) error { if err != nil { return err } + input := accountInput{} return out.Render(func(w io.Writer) error { return renderFullAccount(w, res) - }, accountOutput{ - Input: accountInput{}, - Account: jsonFullAccount(res), - }) + }, newAccountOperationOutput(input, jsonFullAccount(res))) } // Otherwise look up an account with the provided ID @@ -131,14 +126,18 @@ func account(cmd *cobra.Command, args []string) error { if err != nil { return err } + input := accountInput{ + AccountID: args[0], + } return out.Render(func(w io.Writer) error { return renderBasicAccount(w, res) - }, accountOutput{ - Input: accountInput{ - AccountID: args[0], - }, - Account: jsonBasicAccount(res), - }) + }, newAccountOperationOutput(input, jsonBasicAccount(res))) +} + +func newAccountOperationOutput(input accountInput, account jsonAccount) jsonOperationOutput { + return newJSONOperationOutput(input, []jsonOperationResult{ + newJSONOperationResult("", accountKindAccount, input, account), + }, nil) } func jsonFullAccount(fa *users.FullAccount) jsonAccount { diff --git a/cmd/account_test.go b/cmd/account_test.go index d780aee..e66c869 100644 --- a/cmd/account_test.go +++ b/cmd/account_test.go @@ -50,7 +50,14 @@ func TestAccountCurrentJSONOutputsAccount(t *testing.T) { if got.Input.AccountID != "" { t.Fatalf("input.account_id = %q, want empty for current account", got.Input.AccountID) } - account := got.Account + result := got.Results[0] + if result.Kind != accountKindAccount { + t.Fatalf("kind = %q, want account", result.Kind) + } + if result.Input.AccountID != "" { + t.Fatalf("result input.account_id = %q, want empty for current account", result.Input.AccountID) + } + account := result.Result if account.Type != "full" || account.AccountID != "dbid:current" || account.Email != "test@example.com" { t.Fatalf("account = %#v, want current full account", account) } @@ -90,7 +97,14 @@ func TestAccountLookupJSONUsesAccountID(t *testing.T) { if got.Input.AccountID != "dbid:lookup" { t.Fatalf("input.account_id = %q, want dbid:lookup", got.Input.AccountID) } - account := got.Account + result := got.Results[0] + if result.Kind != accountKindAccount { + t.Fatalf("kind = %q, want account", result.Kind) + } + if result.Input.AccountID != "dbid:lookup" { + t.Fatalf("result input.account_id = %q, want dbid:lookup", result.Input.AccountID) + } + account := result.Result if account.Type != "basic" || account.AccountID != "dbid:lookup" || account.Email != "lookup@example.com" { t.Fatalf("account = %#v, want lookup basic account", account) } @@ -137,13 +151,34 @@ func setAccountOutputJSON(t *testing.T, cmd *cobra.Command) { } } -func decodeAccountOutput(t *testing.T, out *bytes.Buffer) accountOutput { +type accountJSONOutput struct { + Input accountInput `json:"input"` + Results []accountJSONResult `json:"results"` + Warnings []jsonWarning `json:"warnings"` +} + +type accountJSONResult struct { + Kind string `json:"kind"` + Input accountInput `json:"input"` + Result jsonAccount `json:"result"` +} + +func decodeAccountOutput(t *testing.T, out *bytes.Buffer) accountJSONOutput { t.Helper() - var got accountOutput - if err := json.NewDecoder(out).Decode(&got); err != nil { + var got accountJSONOutput + if err := json.Unmarshal(out.Bytes(), &got); err != nil { t.Fatalf("decode JSON output: %v\noutput: %s", err, out.String()) } + if got.Warnings == nil { + t.Fatalf("warnings = nil, want empty array") + } + if len(got.Warnings) != 0 { + t.Fatalf("warnings = %+v, want empty", got.Warnings) + } + if len(got.Results) != 1 { + t.Fatalf("results len = %d, want 1", len(got.Results)) + } return got } diff --git a/cmd/completion.go b/cmd/completion.go new file mode 100644 index 0000000..538552a --- /dev/null +++ b/cmd/completion.go @@ -0,0 +1,186 @@ +// Copyright © 2016 Dropbox, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "fmt" + + "github.com/spf13/cobra" +) + +func newCompletionCmd() *cobra.Command { + noDesc := false + programName := RootCmd.Name() + + completionCmd := &cobra.Command{ + Use: "completion [bash|zsh|fish|powershell]", + Short: "Generate the autocompletion script for the specified shell", + Long: completionLong(programName), + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() + }, + } + + addCompletionNoDescFlag := func(cmd *cobra.Command) { + cmd.Flags().BoolVar(&noDesc, "no-descriptions", false, "disable completion descriptions") + } + + bash := &cobra.Command{ + Use: "bash", + Short: "Generate the autocompletion script for bash", + Long: bashCompletionLong(programName), + Args: cobra.NoArgs, + DisableFlagsInUseLine: true, + ValidArgsFunction: cobra.NoFileCompletions, + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Root().GenBashCompletionV2(cmd.Root().OutOrStdout(), !noDesc) + }, + } + addCompletionNoDescFlag(bash) + + zsh := &cobra.Command{ + Use: "zsh", + Short: "Generate the autocompletion script for zsh", + Long: zshCompletionLong(programName), + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + RunE: func(cmd *cobra.Command, args []string) error { + if noDesc { + return cmd.Root().GenZshCompletionNoDesc(cmd.Root().OutOrStdout()) + } + return cmd.Root().GenZshCompletion(cmd.Root().OutOrStdout()) + }, + } + addCompletionNoDescFlag(zsh) + + fish := &cobra.Command{ + Use: "fish", + Short: "Generate the autocompletion script for fish", + Long: fishCompletionLong(programName), + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Root().GenFishCompletion(cmd.Root().OutOrStdout(), !noDesc) + }, + } + addCompletionNoDescFlag(fish) + + powershell := &cobra.Command{ + Use: "powershell", + Short: "Generate the autocompletion script for powershell", + Long: powershellCompletionLong(programName), + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + RunE: func(cmd *cobra.Command, args []string) error { + if noDesc { + return cmd.Root().GenPowerShellCompletion(cmd.Root().OutOrStdout()) + } + return cmd.Root().GenPowerShellCompletionWithDesc(cmd.Root().OutOrStdout()) + }, + } + addCompletionNoDescFlag(powershell) + + completionCmd.AddCommand(bash, zsh, fish, powershell) + return completionCmd +} + +func completionLong(programName string) string { + return fmt.Sprintf(`Generate the autocompletion script for %s for the specified shell. +See each sub-command's help for details on how to use the generated script. +`, programName) +} + +func bashCompletionLong(programName string) string { + return fmt.Sprintf(`Generate the autocompletion script for the bash shell. + +This script depends on the 'bash-completion' package. +If it is not installed already, you can install it via your OS's package manager. + +To load completions in your current shell session: + + source <(%[1]s completion bash) + +To load completions for every new session, execute once: + +#### Linux: + + %[1]s completion bash > /etc/bash_completion.d/%[1]s + +#### macOS: + + %[1]s completion bash > $(brew --prefix)/etc/bash_completion.d/%[1]s + +You will need to start a new shell for this setup to take effect. +`, programName) +} + +func zshCompletionLong(programName string) string { + return fmt.Sprintf(`Generate the autocompletion script for the zsh shell. + +If shell completion is not already enabled in your environment you will need +to enable it. You can execute the following once: + + echo "autoload -U compinit; compinit" >> ~/.zshrc + +To load completions in your current shell session: + + source <(%[1]s completion zsh) + +To load completions for every new session, execute once: + +#### Linux: + + %[1]s completion zsh > "${fpath[1]}/_%[1]s" + +#### macOS: + + %[1]s completion zsh > $(brew --prefix)/share/zsh/site-functions/_%[1]s + +You will need to start a new shell for this setup to take effect. +`, programName) +} + +func fishCompletionLong(programName string) string { + return fmt.Sprintf(`Generate the autocompletion script for the fish shell. + +To load completions in your current shell session: + + %[1]s completion fish | source + +To load completions for every new session, execute once: + + %[1]s completion fish > ~/.config/fish/completions/%[1]s.fish + +You will need to start a new shell for this setup to take effect. +`, programName) +} + +func powershellCompletionLong(programName string) string { + return fmt.Sprintf(`Generate the autocompletion script for powershell. + +To load completions in your current shell session: + + %[1]s completion powershell | Out-String | Invoke-Expression + +To load completions for every new session, add the output of the above command +to your powershell profile. +`, programName) +} + +func init() { + RootCmd.AddCommand(newCompletionCmd()) +} diff --git a/cmd/du.go b/cmd/du.go index 30b8879..f311a36 100644 --- a/cmd/du.go +++ b/cmd/du.go @@ -23,6 +23,8 @@ import ( "github.com/spf13/cobra" ) +type duInput struct{} + type duOutput struct { Used uint64 `json:"used"` Allocation duAllocation `json:"allocation"` @@ -37,6 +39,8 @@ type duAllocation struct { UserWithinTeamSpaceLimitType string `json:"user_within_team_space_limit_type,omitempty"` } +const duKindSpaceUsage = "space_usage" + func du(cmd *cobra.Command, args []string) (err error) { dbx := usersNewFunc(config) usage, err := dbx.GetSpaceUsage() @@ -46,7 +50,7 @@ func du(cmd *cobra.Command, args []string) (err error) { return commandOutput(cmd).Render(func(w io.Writer) error { return renderUsage(w, usage) - }, newDuOutput(usage)) + }, newDuOperationOutput(usage)) } func renderUsage(out io.Writer, usage *users.SpaceUsage) error { @@ -74,6 +78,13 @@ func newDuOutput(usage *users.SpaceUsage) duOutput { } } +func newDuOperationOutput(usage *users.SpaceUsage) jsonOperationOutput { + input := duInput{} + return newJSONOperationOutput(input, []jsonOperationResult{ + newJSONOperationResult("", duKindSpaceUsage, input, newDuOutput(usage)), + }, nil) +} + func newDuAllocation(allocation *users.SpaceAllocation) duAllocation { result := duAllocation{ Type: allocation.Tag, diff --git a/cmd/du_test.go b/cmd/du_test.go index b007afd..0348d59 100644 --- a/cmd/du_test.go +++ b/cmd/du_test.go @@ -50,14 +50,24 @@ func TestDuJSONIndividualAllocation(t *testing.T) { } got := decodeDuOutput(t, stdout) - if got.Used != 1024 { - t.Fatalf("used = %d, want 1024", got.Used) + if len(got.Input) != 0 { + t.Fatalf("input = %#v, want empty object", got.Input) } - if got.Allocation.Type != "individual" || got.Allocation.Allocated == nil || *got.Allocation.Allocated != 2048 { - t.Fatalf("allocation = %#v, want individual allocation", got.Allocation) + result := got.Results[0] + if result.Kind != duKindSpaceUsage { + t.Fatalf("kind = %q, want space_usage", result.Kind) } - if got.Allocation.Used != nil { - t.Fatalf("allocation.used = %#v, want omitted for individual allocation", got.Allocation.Used) + if len(result.Input) != 0 { + t.Fatalf("result input = %#v, want empty object", result.Input) + } + if result.Result.Used != 1024 { + t.Fatalf("used = %d, want 1024", result.Result.Used) + } + if result.Result.Allocation.Type != "individual" || result.Result.Allocation.Allocated == nil || *result.Result.Allocation.Allocated != 2048 { + t.Fatalf("allocation = %#v, want individual allocation", result.Result.Allocation) + } + if result.Result.Allocation.Used != nil { + t.Fatalf("allocation.used = %#v, want omitted for individual allocation", result.Result.Allocation.Used) } } @@ -75,20 +85,24 @@ func TestDuJSONTeamAllocation(t *testing.T) { } got := decodeDuOutput(t, stdout) - if got.Used != 4096 { - t.Fatalf("used = %d, want 4096", got.Used) + result := got.Results[0] + if result.Kind != duKindSpaceUsage { + t.Fatalf("kind = %q, want space_usage", result.Kind) } - if got.Allocation.Type != "team" { - t.Fatalf("allocation.type = %q, want team", got.Allocation.Type) + if result.Result.Used != 4096 { + t.Fatalf("used = %d, want 4096", result.Result.Used) } - if got.Allocation.Allocated == nil || *got.Allocation.Allocated != 8192 { - t.Fatalf("allocation.allocated = %#v, want 8192", got.Allocation.Allocated) + if result.Result.Allocation.Type != "team" { + t.Fatalf("allocation.type = %q, want team", result.Result.Allocation.Type) } - if got.Allocation.Used == nil || *got.Allocation.Used != 2048 { - t.Fatalf("allocation.used = %#v, want 2048", got.Allocation.Used) + if result.Result.Allocation.Allocated == nil || *result.Result.Allocation.Allocated != 8192 { + t.Fatalf("allocation.allocated = %#v, want 8192", result.Result.Allocation.Allocated) } - if got.Allocation.UserWithinTeamSpaceLimitType != "alert_only" { - t.Fatalf("user_within_team_space_limit_type = %q, want alert_only", got.Allocation.UserWithinTeamSpaceLimitType) + if result.Result.Allocation.Used == nil || *result.Result.Allocation.Used != 2048 { + t.Fatalf("allocation.used = %#v, want 2048", result.Result.Allocation.Used) + } + if result.Result.Allocation.UserWithinTeamSpaceLimitType != "alert_only" { + t.Fatalf("user_within_team_space_limit_type = %q, want alert_only", result.Result.Allocation.UserWithinTeamSpaceLimitType) } } @@ -130,13 +144,34 @@ func setDuOutputJSON(t *testing.T, cmd *cobra.Command) { } } -func decodeDuOutput(t *testing.T, out *bytes.Buffer) duOutput { +type duJSONOutput struct { + Input map[string]any `json:"input"` + Results []duJSONResult `json:"results"` + Warnings []jsonWarning `json:"warnings"` +} + +type duJSONResult struct { + Kind string `json:"kind"` + Input map[string]any `json:"input"` + Result duOutput `json:"result"` +} + +func decodeDuOutput(t *testing.T, out *bytes.Buffer) duJSONOutput { t.Helper() - var got duOutput - if err := json.NewDecoder(out).Decode(&got); err != nil { + var got duJSONOutput + if err := json.Unmarshal(out.Bytes(), &got); err != nil { t.Fatalf("decode JSON output: %v\noutput: %s", err, out.String()) } + if got.Warnings == nil { + t.Fatalf("warnings = nil, want empty array") + } + if len(got.Warnings) != 0 { + t.Fatalf("warnings = %+v, want empty", got.Warnings) + } + if len(got.Results) != 1 { + t.Fatalf("results len = %d, want 1", len(got.Results)) + } return got } diff --git a/cmd/root_test.go b/cmd/root_test.go index 045eb3c..73018a2 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,13 +1,16 @@ package cmd import ( + "bytes" "context" + "errors" "os" "path/filepath" "strings" "testing" "time" + "github.com/dropbox/dbxcli/internal/output" "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox" "github.com/spf13/cobra" "golang.org/x/oauth2" @@ -122,6 +125,39 @@ func TestInitDbxRejectsUnsupportedStructuredOutputBeforeAuth(t *testing.T) { } } +func TestCompletionJSONUnsupportedOutputReturnsError(t *testing.T) { + t.Setenv(envAccessToken, "") + t.Setenv(envAuthFile, filepath.Join(t.TempDir(), "missing-auth.json")) + + var stdout bytes.Buffer + var stderr bytes.Buffer + root := &cobra.Command{ + Use: "dbxcli", + SilenceUsage: true, + SilenceErrors: true, + PersistentPreRunE: initDbx, + } + root.SetOut(&stdout) + root.SetErr(&stderr) + root.SetArgs([]string{"completion", "--output=json"}) + root.PersistentFlags().BoolP("verbose", "v", false, "") + root.PersistentFlags().String(outputFlag, "text", "") + root.PersistentFlags().String("as-member", "", "") + root.PersistentFlags().String("domain", "", "") + root.AddCommand(newCompletionCmd()) + + err := root.Execute() + if !errors.Is(err, output.ErrStructuredOutputUnsupported) { + t.Fatalf("error = %v, want structured output unsupported", err) + } + if got := stdout.String(); got != "" { + t.Fatalf("stdout = %q, want no text help output", got) + } + if got := stderr.String(); got != "" { + t.Fatalf("stderr = %q, want empty", got) + } +} + func TestInitDbxStillRequiresAuthForDropboxCommands(t *testing.T) { t.Setenv(envAccessToken, "") t.Setenv(envAuthFile, filepath.Join(t.TempDir(), "missing-auth.json"))