diff --git a/README.md b/README.md index a18f088..84f878f 100644 --- a/README.md +++ b/README.md @@ -168,6 +168,7 @@ $ dbxcli restore --output=json /Reports/old.pdf 015f... Structured success output is rolling out command by command. Currently migrated commands are `version`, `account`, `du`, `ls`, `search`, `revs`, `cp`, `mv`, `put`, `get`, `share-link create`, `share-link list`, `share-link info`, `share-link update`, `share-link revoke`, `share-link download`, `share list folder`, `team info`, `team list-members`, `team list-groups`, `team add-member`, `team remove-member`, `mkdir`, `rm`, and `restore`. Commands that have not been migrated return a JSON error whose `error.message` is `structured output is not supported for this command yet` when used with `--output=json`. 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. Successful JSON payloads use the same `warnings` field. +Current warning codes include `deprecated_command` for deprecated command paths and `skipped_symlink` for symlinks skipped by recursive upload. Successful JSON responses for migrated commands return an `input` object, a `results` array, and a `warnings` array. Result payloads are command-specific. For commands such as `mkdir`, each result reports what happened to the requested path: diff --git a/cmd/account.go b/cmd/account.go index 71c919f..b81984b 100644 --- a/cmd/account.go +++ b/cmd/account.go @@ -15,7 +15,6 @@ package cmd import ( - "errors" "fmt" "io" "text/tabwriter" @@ -102,7 +101,7 @@ func renderBasicAccount(out io.Writer, ba *users.BasicAccount) error { func account(cmd *cobra.Command, args []string) error { if len(args) > 1 { - return errors.New("`account` accepts an optional `id` argument") + return invalidArgumentsError("`account` accepts an optional `id` argument") } dbx := usersNewFunc(config) diff --git a/cmd/add-member.go b/cmd/add-member.go index 72beb01..ab161ac 100644 --- a/cmd/add-member.go +++ b/cmd/add-member.go @@ -15,7 +15,6 @@ package cmd import ( - "errors" "fmt" "io" @@ -25,7 +24,7 @@ import ( func addMember(cmd *cobra.Command, args []string) (err error) { if len(args) != 3 { - return errors.New("`add-member` requires `email`, `first`, and `last` arguments") + return invalidArgumentsError("`add-member` requires `email`, `first`, and `last` arguments") } dbx := teamNewFunc(config) diff --git a/cmd/cp.go b/cmd/cp.go index 6beb4ce..19c9a3f 100644 --- a/cmd/cp.go +++ b/cmd/cp.go @@ -15,7 +15,6 @@ package cmd import ( - "errors" "fmt" "strings" @@ -34,7 +33,7 @@ func cp(cmd *cobra.Command, args []string) error { destination = args[1] argsToCopy = append(argsToCopy, args[0]) } else { - return errors.New("cp requires a source and a destination") + return invalidArgumentsError("cp requires a source and a destination") } var cpErrors []error diff --git a/cmd/get.go b/cmd/get.go index b9f061d..11cd69e 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -65,7 +65,7 @@ type getResult struct { func get(cmd *cobra.Command, args []string) (err error) { if len(args) == 0 || len(args) > 2 { - return errors.New("`get` requires `src` and/or `dst` arguments") + return invalidArgumentsError("`get` requires `src` and/or `dst` arguments") } src, err := validatePath(args[0]) @@ -83,7 +83,7 @@ func get(cmd *cobra.Command, args []string) (err error) { if dst == "-" { if commandOutputFormat(cmd) == output.FormatJSON { - return errors.New("`get --output=json` cannot be used with stdout target `-`") + return invalidArgumentsError("`get --output=json` cannot be used with stdout target `-`") } return getStdout(cmd, src, recursive) } @@ -113,7 +113,7 @@ func get(cmd *cobra.Command, args []string) (err error) { if _, ok := meta.(*files.FolderMetadata); ok { if !recursive { - return fmt.Errorf("%s is a folder (use --recursive to download folders)", src) + return invalidArgumentsErrorf("%s is a folder (use --recursive to download folders)", src) } if f, statErr := os.Stat(dst); statErr == nil && f.IsDir() { dst = filepath.Join(dst, path.Base(src)) @@ -196,7 +196,7 @@ func getOperationResults(results []getResult) []jsonOperationResult { func getStdout(cmd *cobra.Command, src string, recursive bool) error { if recursive { - return errors.New("`get -` cannot be used with --recursive") + return invalidArgumentsError("`get -` cannot be used with --recursive") } dbx := filesNewFunc(config) @@ -204,7 +204,7 @@ func getStdout(cmd *cobra.Command, src string, recursive bool) error { meta, err := dbx.GetMetadata(files.NewGetMetadataArg(src)) if err == nil { if _, ok := meta.(*files.FolderMetadata); ok { - return fmt.Errorf("%s is a folder; cannot download folder to stdout", src) + return invalidArgumentsErrorf("%s is a folder; cannot download folder to stdout", src) } } @@ -213,7 +213,7 @@ func getStdout(cmd *cobra.Command, src string, recursive bool) error { func getWithClient(dbx files.Client, args []string) (err error) { if len(args) == 0 || len(args) > 2 { - return errors.New("`get` requires `src` and/or `dst` arguments") + return invalidArgumentsError("`get` requires `src` and/or `dst` arguments") } src, err := validatePath(args[0]) @@ -342,7 +342,7 @@ func ensureLocalDirectoryResult(source, target string, metadata files.IsMetadata status := getStatusCreated if info, err := os.Stat(target); err == nil { if !info.IsDir() { - return getResult{}, fmt.Errorf("path exists and is not a folder: %s", target) + return getResult{}, pathConflictErrorf("path exists and is not a folder: %s", target) } status = getStatusExisting } else if !os.IsNotExist(err) { diff --git a/cmd/get_test.go b/cmd/get_test.go index 6c10962..84d3a3d 100644 --- a/cmd/get_test.go +++ b/cmd/get_test.go @@ -509,6 +509,9 @@ func TestGetFolderWithoutRecursiveFlag(t *testing.T) { if !strings.Contains(err.Error(), "--recursive") { t.Errorf("error = %q, want mention of --recursive", err.Error()) } + if got, want := jsonErrorCode(err), jsonErrorCodeInvalidArguments; got != want { + t.Fatalf("jsonErrorCode = %q, want %q", got, want) + } } func TestGetRecursiveCommandGetsMetadataThenListsFolder(t *testing.T) { @@ -718,11 +721,34 @@ func TestGetJSONRejectsStdoutTarget(t *testing.T) { if !strings.Contains(err.Error(), "stdout target") { t.Fatalf("error = %q, want stdout target", err.Error()) } + if got, want := jsonErrorCode(err), jsonErrorCodeInvalidArguments; got != want { + t.Fatalf("jsonErrorCode = %q, want %q", got, want) + } if stdout.Len() != 0 { t.Fatalf("stdout = %q, want empty", stdout.String()) } } +func TestGetStdoutFolderErrorHasInvalidArgumentsCode(t *testing.T) { + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return getTestFolderMetadata(arg.Path), nil + }, + } + stubFilesClient(t, mock) + + err := getStdout(&cobra.Command{}, "/remote-folder", false) + if err == nil { + t.Fatal("expected folder stdout error") + } + if !strings.Contains(err.Error(), "cannot download folder to stdout") { + t.Fatalf("error = %q, want stdout folder error", err.Error()) + } + if got, want := jsonErrorCode(err), jsonErrorCodeInvalidArguments; got != want { + t.Fatalf("jsonErrorCode = %q, want %q", got, want) + } +} + func TestGetTextStdoutTargetWritesOnlyFileBytes(t *testing.T) { mock := &mockFilesClient{ getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { diff --git a/cmd/json_contract_test.go b/cmd/json_contract_test.go new file mode 100644 index 0000000..af5add7 --- /dev/null +++ b/cmd/json_contract_test.go @@ -0,0 +1,130 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "errors" + "sort" + "strings" + "testing" + + "github.com/dropbox/dbxcli/internal/output" + "github.com/spf13/cobra" +) + +func TestStructuredOutputCommandAudit(t *testing.T) { + got := structuredOutputCommandPaths(RootCmd) + got = append(got, NewVersionCommand("test").Name()) + sort.Strings(got) + + want := []string{ + "account", + "cp", + "du", + "get", + "ls", + "mkdir", + "mv", + "put", + "restore", + "revs", + "rm", + "search", + "share list folder", + "share list link", + "share-link create", + "share-link download", + "share-link info", + "share-link list", + "share-link revoke", + "share-link update", + "team add-member", + "team info", + "team list-groups", + "team list-members", + "team remove-member", + "version", + } + sort.Strings(want) + + if len(got) != len(want) { + t.Fatalf("structured commands = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("structured commands = %v, want %v", got, want) + } + } +} + +func structuredOutputCommandPaths(root *cobra.Command) []string { + var paths []string + var walk func(*cobra.Command, []string) + walk = func(cmd *cobra.Command, parents []string) { + parts := parents + if cmd != root { + parts = append(append([]string{}, parents...), cmd.Name()) + if commandSupportsStructuredOutput(cmd) { + paths = append(paths, strings.Join(parts, " ")) + } + } + for _, child := range cmd.Commands() { + walk(child, parts) + } + } + walk(root, nil) + return paths +} + +func TestJSONOperationOutputContractShape(t *testing.T) { + encoded, err := json.Marshal(newJSONOperationOutput(nil, nil, nil)) + if err != nil { + t.Fatalf("marshal operation output: %v", err) + } + + var raw map[string]json.RawMessage + if err := json.Unmarshal(encoded, &raw); err != nil { + t.Fatalf("decode operation output: %v", err) + } + for _, key := range []string{"input", "results", "warnings"} { + if _, ok := raw[key]; !ok { + t.Fatalf("operation output = %s, missing %q", encoded, key) + } + } + if len(raw) != 3 { + t.Fatalf("operation output = %s, want only input/results/warnings", encoded) + } +} + +func TestUnsupportedCommandsReturnJSONErrorEnvelope(t *testing.T) { + for _, name := range []string{"login", "logout", "completion"} { + t.Run(name, func(t *testing.T) { + var stdout, stderr bytes.Buffer + cmd := &cobra.Command{Use: name} + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.Flags().String(outputFlag, "json", "") + + err := validateOutputFormat(cmd) + if !errors.Is(err, output.ErrStructuredOutputUnsupported) { + t.Fatalf("validateOutputFormat error = %v, want structured output unsupported", err) + } + + renderCommandError(cmd, err) + if stderr.Len() != 0 { + t.Fatalf("stderr = %q, want empty", stderr.String()) + } + + got := decodeJSONErrorResponse(t, stdout.String()) + if got.OK { + t.Fatalf("ok = true, want false") + } + if got.Error.Code != jsonErrorCodeStructuredOutputUnsupported { + t.Fatalf("code = %q, want %q", got.Error.Code, jsonErrorCodeStructuredOutputUnsupported) + } + if got.Warnings == nil || len(got.Warnings) != 0 { + t.Fatalf("warnings = %+v, want empty array", got.Warnings) + } + }) + } +} diff --git a/cmd/json_output.go b/cmd/json_output.go index 0d0f344..85486ec 100644 --- a/cmd/json_output.go +++ b/cmd/json_output.go @@ -19,6 +19,11 @@ type jsonWarning struct { Path string `json:"path,omitempty"` } +const ( + jsonWarningCodeDeprecatedCommand = "deprecated_command" + jsonWarningCodeSkippedSymlink = "skipped_symlink" +) + type jsonOperationOutput struct { Input any `json:"input"` Results []jsonOperationResult `json:"results"` @@ -52,7 +57,11 @@ func newJSONOperationOutput(input any, results []jsonOperationResult, warnings [ } func renderJSONOperationOutput(cmd *cobra.Command, input any, results []jsonOperationResult) error { - return commandOutput(cmd).Render(nil, newJSONOperationOutput(input, results, nil)) + return renderJSONOperationOutputWithWarnings(cmd, input, results, nil) +} + +func renderJSONOperationOutputWithWarnings(cmd *cobra.Command, input any, results []jsonOperationResult, warnings []jsonWarning) error { + return commandOutput(cmd).Render(nil, newJSONOperationOutput(input, results, warnings)) } func newJSONOperationResult(status, kind string, input any, result any) jsonOperationResult { diff --git a/cmd/mkdir.go b/cmd/mkdir.go index f6c8413..ca0d0bd 100644 --- a/cmd/mkdir.go +++ b/cmd/mkdir.go @@ -16,7 +16,6 @@ package cmd import ( "errors" - "fmt" "strings" "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files" @@ -44,7 +43,7 @@ type mkdirResult struct { func mkdir(cmd *cobra.Command, args []string) (err error) { if len(args) != 1 { - return errors.New("`mkdir` requires a `directory` argument") + return invalidArgumentsError("`mkdir` requires a `directory` argument") } dst, err := validatePath(args[0]) @@ -77,7 +76,7 @@ func mkdir(cmd *cobra.Command, args []string) (err error) { } status = mkdirStatusExisting case ok && (conflictTag == files.WriteConflictErrorFile || conflictTag == files.WriteConflictErrorFileAncestor): - return fmt.Errorf("path exists and is not a folder: %s", dst) + return pathConflictErrorf("path exists and is not a folder: %s", dst) case ok: return err case isConflictError(err): @@ -110,7 +109,7 @@ func existingFolderMetadata(dbx files.Client, dst string) (*files.FolderMetadata } folder, ok := metadata.(*files.FolderMetadata) if !ok || folder == nil { - return nil, fmt.Errorf("path exists and is not a folder: %s", dst) + return nil, pathConflictErrorf("path exists and is not a folder: %s", dst) } return folder, nil } diff --git a/cmd/mv.go b/cmd/mv.go index 7cbbfa2..74bbb9a 100644 --- a/cmd/mv.go +++ b/cmd/mv.go @@ -33,7 +33,7 @@ func mv(cmd *cobra.Command, args []string) error { destination = args[1] argsToMove = append(argsToMove, args[0]) } else { - return fmt.Errorf("mv command requires a source and a destination") + return invalidArgumentsError("mv command requires a source and a destination") } var mvErrors []error diff --git a/cmd/output.go b/cmd/output.go index 3e0cdf5..224f609 100644 --- a/cmd/output.go +++ b/cmd/output.go @@ -12,8 +12,64 @@ import ( const ( outputFlag = "output" structuredOutputSupportedAnnotation = "dbxcli.supportsStructuredOutput" + + jsonErrorCodeCommandFailed = "command_failed" + jsonErrorCodeInvalidArguments = "invalid_arguments" + jsonErrorCodePathConflict = "path_conflict" + jsonErrorCodeStructuredOutputUnsupported = "structured_output_unsupported" + jsonErrorCodeUnknownCommand = "unknown_command" + jsonErrorCodeUnknownFlag = "unknown_flag" + jsonErrorCodeUnsupportedOutputFormat = "unsupported_output_format" ) +type jsonCodedError interface { + error + JSONErrorCode() string +} + +type codedError struct { + code string + err error +} + +func (e codedError) Error() string { + return e.err.Error() +} + +func (e codedError) Unwrap() error { + return e.err +} + +func (e codedError) JSONErrorCode() string { + return e.code +} + +func newCodedError(code string, err error) error { + if err == nil { + return nil + } + return codedError{ + code: code, + err: err, + } +} + +func invalidArgumentsError(message string) error { + return newCodedError(jsonErrorCodeInvalidArguments, errors.New(message)) +} + +func invalidArgumentsErrorf(format string, args ...any) error { + return newCodedError(jsonErrorCodeInvalidArguments, fmt.Errorf(format, args...)) +} + +func pathConflictErrorf(format string, args ...any) error { + return newCodedError(jsonErrorCodePathConflict, fmt.Errorf(format, args...)) +} + +func unsupportedOutputFormatErrorf(format string, args ...any) error { + return newCodedError(jsonErrorCodeUnsupportedOutputFormat, fmt.Errorf(format, args...)) +} + func commandOutput(cmd *cobra.Command) *output.Renderer { if cmd == nil { return output.New(nil, nil, output.FormatText) @@ -61,7 +117,7 @@ func parseOutputFormat(value string) (output.Format, error) { case output.FormatJSON: return output.FormatJSON, nil default: - return "", fmt.Errorf("unsupported output format %q: use text or json", value) + return "", unsupportedOutputFormatErrorf("unsupported output format %q: use text or json", value) } } @@ -130,7 +186,7 @@ func renderCommandErrorWithJSON(cmd *cobra.Command, err error, forceJSON bool) { } _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", err) - if jsonErrorCode(err) == "unknown_command" { + if jsonErrorCode(err) == jsonErrorCodeUnknownCommand { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Run '%s --help' for usage.\n", cmd.CommandPath()) } } @@ -164,27 +220,24 @@ func outputJSONRequested(args []string) bool { } // jsonErrorCode derives stable script-facing codes from existing CLI errors. -// If a matched error message changes, update this mapping with it. +// Prefer coded errors for dbxcli-owned validation failures. String matching is +// kept only for Cobra-generated unknown command/flag errors and legacy fallback. func jsonErrorCode(err error) string { + var coded jsonCodedError + if errors.As(err, &coded) { + return coded.JSONErrorCode() + } if errors.Is(err, output.ErrStructuredOutputUnsupported) { - return "structured_output_unsupported" + return jsonErrorCodeStructuredOutputUnsupported } message := err.Error() switch { - case strings.Contains(message, "unsupported output format"): - return "unsupported_output_format" case strings.Contains(message, "unknown command"): - return "unknown_command" + return jsonErrorCodeUnknownCommand case strings.Contains(message, "unknown flag"): - return "unknown_flag" - case strings.Contains(message, "path exists and is not a folder"): - return "path_conflict" - case strings.Contains(message, "requires "): - return "invalid_arguments" - case strings.Contains(message, "accepts an optional"): - return "invalid_arguments" + return jsonErrorCodeUnknownFlag default: - return "command_failed" + return jsonErrorCodeCommandFailed } } diff --git a/cmd/output_test.go b/cmd/output_test.go index 41dc6af..86f30cd 100644 --- a/cmd/output_test.go +++ b/cmd/output_test.go @@ -594,24 +594,52 @@ func TestOutputJSONRequested(t *testing.T) { } } -func TestJSONErrorCodePathConflict(t *testing.T) { - err := errors.New("path exists and is not a folder: /file") - if got, want := jsonErrorCode(err), "path_conflict"; got != want { - t.Fatalf("jsonErrorCode = %q, want %q", got, want) +func TestJSONErrorCodeUsesCodedErrors(t *testing.T) { + tests := []struct { + name string + err error + want string + }{ + { + name: "path conflict", + err: pathConflictErrorf("path exists and is not a folder: /file"), + want: jsonErrorCodePathConflict, + }, + { + name: "optional argument validation", + err: invalidArgumentsError("`account` accepts an optional `id` argument"), + want: jsonErrorCodeInvalidArguments, + }, + { + name: "required argument validation", + err: invalidArgumentsError("`add-member` requires `email`, `first`, and `last` arguments"), + want: jsonErrorCodeInvalidArguments, + }, + { + name: "unsupported output format", + err: unsupportedOutputFormatErrorf("unsupported output format %q: use text or json", "yaml"), + want: jsonErrorCodeUnsupportedOutputFormat, + }, } -} -func TestJSONErrorCodeOptionalArgumentValidation(t *testing.T) { - err := errors.New("`account` accepts an optional `id` argument") - if got, want := jsonErrorCode(err), "invalid_arguments"; got != want { - t.Fatalf("jsonErrorCode = %q, want %q", got, want) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := jsonErrorCode(tt.err); got != tt.want { + t.Fatalf("jsonErrorCode = %q, want %q", got, tt.want) + } + }) } } -func TestJSONErrorCodeRequiredArgumentValidation(t *testing.T) { - err := errors.New("`add-member` requires `email`, `first`, and `last` arguments") - if got, want := jsonErrorCode(err), "invalid_arguments"; got != want { - t.Fatalf("jsonErrorCode = %q, want %q", got, want) +func TestJSONErrorCodeDoesNotClassifyPlainValidationStrings(t *testing.T) { + for _, err := range []error{ + errors.New("path exists and is not a folder: /file"), + errors.New("Dropbox API requires team admin permissions"), + errors.New("`account` accepts an optional `id` argument"), + } { + if got := jsonErrorCode(err); got != jsonErrorCodeCommandFailed { + t.Fatalf("jsonErrorCode(%q) = %q, want %q", err.Error(), got, jsonErrorCodeCommandFailed) + } } } diff --git a/cmd/put.go b/cmd/put.go index 257831f..3a9551f 100644 --- a/cmd/put.go +++ b/cmd/put.go @@ -260,7 +260,11 @@ func newPutResult(status, kind, source, target string, metadata files.IsMetadata } func renderPutResults(cmd *cobra.Command, input putCommandInput, results []putResult) error { - return renderJSONOperationOutput(cmd, input, putOperationResults(results)) + return renderPutResultsWithWarnings(cmd, input, results, nil) +} + +func renderPutResultsWithWarnings(cmd *cobra.Command, input putCommandInput, results []putResult, warnings []jsonWarning) error { + return renderJSONOperationOutputWithWarnings(cmd, input, putOperationResults(results), warnings) } func putOperationResults(results []putResult) []jsonOperationResult { @@ -277,7 +281,7 @@ func putOperationResults(results []putResult) []jsonOperationResult { func put(cmd *cobra.Command, args []string) (err error) { if len(args) == 0 || len(args) > 2 { - return errors.New("`put` requires `src` and/or `dst` arguments") + return invalidArgumentsError("`put` requires `src` and/or `dst` arguments") } opts, err := parsePutOptions(cmd) @@ -299,7 +303,7 @@ func put(cmd *cobra.Command, args []string) (err error) { } if srcInfo.IsDir() && !recursive { - return fmt.Errorf("%s is a directory (use --recursive to upload directories)", src) + return invalidArgumentsErrorf("%s is a directory (use --recursive to upload directories)", src) } // Default `dst` to the base segment of the source path; use the second argument if provided. @@ -321,17 +325,17 @@ func put(cmd *cobra.Command, args []string) (err error) { if commandOutputFormat(cmd) == output.FormatText { return putRecursive(src, dst, opts) } - results, err := putRecursiveWithResults(src, dst, opts) + results, warnings, err := putRecursiveWithResults(src, dst, opts) if err != nil { return err } - return renderPutResults(cmd, putCommandInput{ + return renderPutResultsWithWarnings(cmd, putCommandInput{ Source: src, Target: dst, Recursive: true, IfExists: opts.ifExists, Stdin: false, - }, results) + }, results, warnings) } result, err := putFileWithResult(src, dst, opts) @@ -349,15 +353,15 @@ func put(cmd *cobra.Command, args []string) (err error) { func putStdin(cmd *cobra.Command, args []string, opts putOptions, recursive bool) error { if len(args) < 2 { - return errors.New("`put -` requires an explicit target path") + return invalidArgumentsError("`put -` requires an explicit target path") } if recursive { - return errors.New("`put -` cannot be used with --recursive") + return invalidArgumentsError("`put -` cannot be used with --recursive") } dst := args[1] if strings.HasSuffix(dst, "/") { - return fmt.Errorf("cannot upload stdin to directory target %q; provide a full Dropbox file path", dst) + return invalidArgumentsErrorf("cannot upload stdin to directory target %q; provide a full Dropbox file path", dst) } dstPath, err := validatePath(dst) @@ -436,7 +440,7 @@ func parsePutOptions(cmd *cobra.Command) (putOptions, error) { return putOptions{}, err } if chunkSize%(1<<22) != 0 { - return putOptions{}, errors.New("`put` requires chunk size to be multiple of 4MiB") + return putOptions{}, invalidArgumentsError("`put` requires chunk size to be multiple of 4MiB") } workers, err := cmd.Flags().GetInt("workers") if err != nil { @@ -476,7 +480,7 @@ func normalizePutIfExists(ifExists string) (string, error) { case putIfExistsOverwrite, putIfExistsSkip, putIfExistsFail: return ifExists, nil default: - return "", fmt.Errorf("invalid --if-exists %q (use overwrite, skip, or fail)", ifExists) + return "", invalidArgumentsErrorf("invalid --if-exists %q (use overwrite, skip, or fail)", ifExists) } } @@ -568,7 +572,7 @@ func checkPutStdinDestination(dbx files.Client, dst string, ifExists string) (pu return putDestinationUpload, nil, nil } if _, ok := meta.(*files.FolderMetadata); ok { - return putDestinationUpload, nil, fmt.Errorf("cannot upload stdin to folder %q; provide a full Dropbox file path", dst) + return putDestinationUpload, nil, invalidArgumentsErrorf("cannot upload stdin to folder %q; provide a full Dropbox file path", dst) } return actionForExistingDestination(dst, ifExists, meta) } @@ -590,7 +594,7 @@ func checkPutDestination(dbx files.Client, dst string, ifExists string) (putDest return putDestinationUpload, nil, nil } if _, ok := meta.(*files.FolderMetadata); ok { - return putDestinationUpload, nil, fmt.Errorf("destination %q is a folder", dst) + return putDestinationUpload, nil, pathConflictErrorf("destination %q is a folder", dst) } return actionForExistingDestination(dst, ifExists, meta) } @@ -600,7 +604,7 @@ func actionForExistingDestination(dst string, ifExists string, metadata files.Is case putIfExistsSkip: return putDestinationSkip, metadata, nil case putIfExistsFail: - return putDestinationUpload, nil, fmt.Errorf("destination %q already exists", dst) + return putDestinationUpload, nil, pathConflictErrorf("destination %q already exists", dst) default: return putDestinationUpload, nil, nil } @@ -702,17 +706,18 @@ func putErrorOutput(opts putOptions) io.Writer { } func putRecursive(src, dst string, opts putOptions) error { - _, err := putRecursiveInternal(src, dst, opts, false) + _, _, err := putRecursiveInternal(src, dst, opts, false) return err } -func putRecursiveWithResults(src, dst string, opts putOptions) ([]putResult, error) { +func putRecursiveWithResults(src, dst string, opts putOptions) ([]putResult, []jsonWarning, error) { return putRecursiveInternal(src, dst, opts, true) } -func putRecursiveInternal(src, dst string, opts putOptions, collectResults bool) ([]putResult, error) { +func putRecursiveInternal(src, dst string, opts putOptions, collectResults bool) ([]putResult, []jsonWarning, error) { src = filepath.Clean(src) var results []putResult + var warnings []jsonWarning var uploadErrors []error dirsWithFiles := make(map[string]bool) @@ -724,6 +729,13 @@ func putRecursiveInternal(src, dst string, opts putOptions, collectResults bool) return nil } if !d.Type().IsRegular() { + if collectResults && d.Type()&os.ModeSymlink != 0 { + warnings = append(warnings, jsonWarning{ + Code: jsonWarningCodeSkippedSymlink, + Message: "skipped symlink during recursive upload", + Path: filePath, + }) + } return nil } @@ -753,7 +765,7 @@ func putRecursiveInternal(src, dst string, opts putOptions, collectResults bool) return nil }) if err != nil { - return nil, err + return nil, nil, err } dbx := filesNewFunc(config) @@ -809,13 +821,13 @@ func putRecursiveInternal(src, dst string, opts putOptions, collectResults bool) return nil }) if err != nil { - return nil, err + return nil, nil, err } if len(uploadErrors) > 0 { - return nil, fmt.Errorf("failed to upload %d file(s): %v", len(uploadErrors), uploadErrors[0]) + return nil, nil, fmt.Errorf("failed to upload %d file(s): %v", len(uploadErrors), uploadErrors[0]) } - return results, nil + return results, warnings, nil } func putDirectory(dbx files.Client, dst string) error { @@ -851,7 +863,7 @@ func putDirectoryConflictError(dst string, err error) error { case ok && conflictTag == files.WriteConflictErrorFolder: return nil case ok && (conflictTag == files.WriteConflictErrorFile || conflictTag == files.WriteConflictErrorFileAncestor): - return fmt.Errorf("path exists and is not a folder: %s", dst) + return pathConflictErrorf("path exists and is not a folder: %s", dst) case ok: return err case isConflictError(err): diff --git a/cmd/put_test.go b/cmd/put_test.go index f24ac79..fc506a7 100644 --- a/cmd/put_test.go +++ b/cmd/put_test.go @@ -273,6 +273,16 @@ type putOutputData struct { func decodePutOutput(t *testing.T, stdout *bytes.Buffer) putOutputData { t.Helper() + got := decodePutOutputWithWarnings(t, stdout) + if len(got.Warnings) != 0 { + t.Fatalf("warnings = %+v, want empty", got.Warnings) + } + return got +} + +func decodePutOutputWithWarnings(t *testing.T, stdout *bytes.Buffer) putOutputData { + t.Helper() + var got putOutputData if err := json.Unmarshal(stdout.Bytes(), &got); err != nil { t.Fatalf("decode put JSON output: %v\noutput: %s", err, stdout.String()) @@ -280,9 +290,6 @@ func decodePutOutput(t *testing.T, stdout *bytes.Buffer) putOutputData { if got.Warnings == nil { t.Fatalf("warnings = nil, want empty array") } - if len(got.Warnings) != 0 { - t.Fatalf("warnings = %+v, want empty", got.Warnings) - } return got } @@ -947,6 +954,63 @@ func TestPutJSONRecursiveOutputsDirectoryAndFileResults(t *testing.T) { } } +func TestPutJSONRecursiveWarnsForSkippedSymlink(t *testing.T) { + dir := t.TempDir() + realPath := filepath.Join(dir, "real.txt") + linkPath := filepath.Join(dir, "link.txt") + if err := os.WriteFile(realPath, []byte("real"), 0644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(realPath, linkPath); err != nil { + t.Fatal(err) + } + + mock := &mockFilesClient{ + uploadFn: func(arg *files.UploadArg, content io.Reader) (*files.FileMetadata, error) { + if arg.Path != "/remote/real.txt" { + t.Fatalf("upload path = %q, want /remote/real.txt", arg.Path) + } + if _, err := io.ReadAll(content); err != nil { + t.Fatal(err) + } + return putFileMetadata(arg.Path, 4), nil + }, + createFolderV2Fn: func(arg *files.CreateFolderArg) (*files.CreateFolderResult, error) { + return files.NewCreateFolderResult(putFolderMetadata(arg.Path)), nil + }, + } + stubFilesClient(t, mock) + + var stdout bytes.Buffer + cmd := testPutJSONCmd(&stdout, nil) + if err := cmd.Flags().Set("recursive", "true"); err != nil { + t.Fatal(err) + } + if err := put(cmd, []string{dir, "/remote"}); err != nil { + t.Fatalf("put error: %v", err) + } + + got := decodePutOutputWithWarnings(t, &stdout) + if len(got.Warnings) != 1 { + t.Fatalf("warnings = %+v, want one skipped symlink warning", got.Warnings) + } + warning := got.Warnings[0] + if warning.Code != jsonWarningCodeSkippedSymlink || warning.Path != linkPath { + t.Fatalf("warning = %+v, want skipped symlink at %s", warning, linkPath) + } + if !strings.Contains(warning.Message, "skipped symlink") { + t.Fatalf("warning message = %q, want skipped symlink", warning.Message) + } + for _, result := range got.Results { + if result.Input.Source == linkPath || result.Input.Target == "/remote/link.txt" { + t.Fatalf("symlink should not be uploaded: %+v", result) + } + } + if !json.Valid(stdout.Bytes()) { + t.Fatalf("stdout is not valid JSON: %s", stdout.String()) + } +} + func TestPutJSONRecursiveFailsWhenDirectoryTargetIsExistingFile(t *testing.T) { dir := t.TempDir() diff --git a/cmd/remove-member.go b/cmd/remove-member.go index 3c6c27d..d1c384d 100644 --- a/cmd/remove-member.go +++ b/cmd/remove-member.go @@ -15,7 +15,6 @@ package cmd import ( - "errors" "fmt" "io" @@ -26,7 +25,7 @@ import ( func removeMember(cmd *cobra.Command, args []string) (err error) { if len(args) != 1 { - return errors.New("`remove-member` requires an `email` argument") + return invalidArgumentsError("`remove-member` requires an `email` argument") } dbx := teamNewFunc(config) diff --git a/cmd/restore.go b/cmd/restore.go index 4a3bfcf..1100f2d 100644 --- a/cmd/restore.go +++ b/cmd/restore.go @@ -15,7 +15,6 @@ package cmd import ( - "errors" "fmt" "io" "time" @@ -43,7 +42,7 @@ type restoreResult struct { func restore(cmd *cobra.Command, args []string) (err error) { if len(args) != 2 { - return errors.New("`restore` requires `target-path` and `revision` arguments") + return invalidArgumentsError("`restore` requires `target-path` and `revision` arguments") } path, err := validatePath(args[0]) diff --git a/cmd/revs.go b/cmd/revs.go index ce5aebe..6035781 100644 --- a/cmd/revs.go +++ b/cmd/revs.go @@ -15,7 +15,6 @@ package cmd import ( - "errors" "fmt" "io" "text/tabwriter" @@ -36,7 +35,7 @@ const revsJSONStatusRevision = "revision" func revs(cmd *cobra.Command, args []string) (err error) { if len(args) != 1 { - return errors.New("`revs` requires a `file` argument") + return invalidArgumentsError("`revs` requires a `file` argument") } path, err := validatePath(args[0]) diff --git a/cmd/rm.go b/cmd/rm.go index b187453..741f49f 100644 --- a/cmd/rm.go +++ b/cmd/rm.go @@ -15,7 +15,6 @@ package cmd import ( - "errors" "fmt" "io" @@ -50,7 +49,7 @@ type removeResult struct { func rm(cmd *cobra.Command, args []string) error { if len(args) < 1 { - return errors.New("rm: missing operand") + return invalidArgumentsError("rm: missing operand") } opts, err := parseRemoveOptions(cmd) @@ -134,7 +133,7 @@ func validateRemoveTargets(dbx files.Client, args []string, opts removeOptions) return nil, err } if len(res.Entries) != 0 { - return nil, fmt.Errorf("rm: cannot remove ā€˜%s’: Directory not empty, use `--force`/`-f` or `--recursive`/`-r` to proceed", path) + return nil, invalidArgumentsErrorf("rm: cannot remove ā€˜%s’: Directory not empty, use `--force`/`-f` or `--recursive`/`-r` to proceed", path) } } targets = append(targets, removeTarget{path: path, metadata: pathMetaData}) diff --git a/cmd/rm_test.go b/cmd/rm_test.go index 1155600..31f4499 100644 --- a/cmd/rm_test.go +++ b/cmd/rm_test.go @@ -166,6 +166,9 @@ func TestRmNonEmptyFolderRequiresRecursiveOrForce(t *testing.T) { if !strings.Contains(err.Error(), "--recursive") || !strings.Contains(err.Error(), "--force") { t.Fatalf("error = %q, want recursive and force guidance", err.Error()) } + if got, want := jsonErrorCode(err), jsonErrorCodeInvalidArguments; got != want { + t.Fatalf("jsonErrorCode = %q, want %q", got, want) + } if deleteCalled { t.Fatal("delete called after validation failure") } diff --git a/cmd/search.go b/cmd/search.go index 8212df0..c1750c3 100644 --- a/cmd/search.go +++ b/cmd/search.go @@ -15,7 +15,6 @@ package cmd import ( - "errors" "fmt" "io" "strings" @@ -40,14 +39,14 @@ const searchJSONStatusFound = "found" func search(cmd *cobra.Command, args []string) (err error) { if len(args) == 0 { - return errors.New("`search` requires a `query` argument") + return invalidArgumentsError("`search` requires a `query` argument") } var scope string if len(args) == 2 { scope = args[1] if !strings.HasPrefix(scope, "/") { - return errors.New("`search` `path-scope` must begin with \"/\"") + return invalidArgumentsError("`search` `path-scope` must begin with \"/\"") } } diff --git a/cmd/share-list-links.go b/cmd/share-list-links.go index a552ede..95623bc 100644 --- a/cmd/share-list-links.go +++ b/cmd/share-list-links.go @@ -29,12 +29,19 @@ type shareLinkListInput struct { } func shareListLinks(cmd *cobra.Command, args []string) (err error) { - return shareLinkList(cmd, args) + return shareLinkListWithWarnings(cmd, args, []jsonWarning{{ + Code: jsonWarningCodeDeprecatedCommand, + Message: "use `dbxcli share-link list` instead", + }}) } func shareLinkList(cmd *cobra.Command, args []string) error { + return shareLinkListWithWarnings(cmd, args, nil) +} + +func shareLinkListWithWarnings(cmd *cobra.Command, args []string, warnings []jsonWarning) error { if len(args) > 1 { - return errors.New("`share-link list` accepts at most one `path` argument") + return invalidArgumentsError("`share-link list` accepts at most one `path` argument") } arg := sharing.NewListSharedLinksArg() @@ -72,7 +79,7 @@ func shareLinkList(cmd *cobra.Command, args []string) error { DirectOnly: arg.DirectOnly, }, shareLinkJSONOperationResults(shareLinkJSONStatusListed, entries), - nil, + warnings, )) } diff --git a/cmd/share_link_create.go b/cmd/share_link_create.go index e4ec53f..6322ac5 100644 --- a/cmd/share_link_create.go +++ b/cmd/share_link_create.go @@ -49,7 +49,7 @@ type shareLinkCreateInput struct { func shareLinkCreate(cmd *cobra.Command, args []string) error { if len(args) != 1 { - return errors.New("`share-link create` requires a `path` argument") + return invalidArgumentsError("`share-link create` requires a `path` argument") } path, err := validatePath(args[0]) @@ -57,7 +57,7 @@ func shareLinkCreate(cmd *cobra.Command, args []string) error { return err } if path == "" { - return errors.New("cannot create a shared link for Dropbox root") + return invalidArgumentsError("cannot create a shared link for Dropbox root") } opts, err := parseShareLinkCreateOptions(cmd) @@ -203,10 +203,10 @@ func parseShareLinkCreateOptions(cmd *cobra.Command) (shareLinkCreateOptions, er opts.password = password if opts.expires != nil && opts.removeExpiration { - return opts, errors.New("`--expires` and `--remove-expiration` cannot be used together") + return opts, invalidArgumentsError("`--expires` and `--remove-expiration` cannot be used together") } if opts.allowDownload && opts.disallowDownload { - return opts, errors.New("`--allow-download` and `--disallow-download` cannot be used together") + return opts, invalidArgumentsError("`--allow-download` and `--disallow-download` cannot be used together") } return opts, nil @@ -214,7 +214,7 @@ func parseShareLinkCreateOptions(cmd *cobra.Command) (shareLinkCreateOptions, er func applyExistingSharedLinkCreateOptions(dbx sharedLinkClient, link sharing.IsSharedLinkMetadata, opts shareLinkCreateOptions) (sharing.IsSharedLinkMetadata, error) { if opts.access != nil { - return nil, errors.New("cannot apply `--access` because the shared link already exists") + return nil, invalidArgumentsError("cannot apply `--access` because the shared link already exists") } if opts.expires == nil && !opts.removeExpiration && !opts.allowDownload && !opts.disallowDownload && opts.audience == nil && !opts.password.set { return link, nil @@ -409,7 +409,7 @@ func shareLinkExpiresFlag(cmd *cobra.Command) (*time.Time, error) { } parsed, err := time.Parse(time.RFC3339, value) if err != nil { - return nil, fmt.Errorf("invalid --expires %q: use RFC3339 timestamp", value) + return nil, invalidArgumentsErrorf("invalid --expires %q: use RFC3339 timestamp", value) } return &parsed, nil } @@ -427,7 +427,7 @@ func shareLinkAccessFlag(cmd *cobra.Command) (*sharing.RequestedLinkAccessLevel, case sharing.RequestedLinkAccessLevelMax: return requestedLinkAccessLevel(sharing.RequestedLinkAccessLevelMax), nil default: - return nil, fmt.Errorf("invalid --access %q: use viewer, editor, or max", value) + return nil, invalidArgumentsErrorf("invalid --access %q: use viewer, editor, or max", value) } } @@ -450,7 +450,7 @@ func shareLinkAudienceFlag(cmd *cobra.Command) (*sharing.LinkAudience, error) { case "no-one": return linkAudience(sharing.LinkAudienceNoOne), nil default: - return nil, fmt.Errorf("invalid --audience %q: use public, team, members, or no-one", value) + return nil, invalidArgumentsErrorf("invalid --audience %q: use public, team, members, or no-one", value) } } diff --git a/cmd/share_link_download.go b/cmd/share_link_download.go index a62cf34..2d0a007 100644 --- a/cmd/share_link_download.go +++ b/cmd/share_link_download.go @@ -52,19 +52,19 @@ type shareLinkDownloadResult struct { func shareLinkDownload(cmd *cobra.Command, args []string) error { if len(args) == 0 || len(args) > 2 { - return errors.New("`share-link download` requires a `url` and optional `target` argument") + return invalidArgumentsError("`share-link download` requires a `url` and optional `target` argument") } url := args[0] if url == "" { - return errors.New("`share-link download` requires a non-empty URL") + return invalidArgumentsError("`share-link download` requires a non-empty URL") } target := "" if len(args) == 2 { target = args[1] if target == "" { - return errors.New("`share-link download` requires a non-empty target") + return invalidArgumentsError("`share-link download` requires a non-empty target") } } @@ -77,7 +77,7 @@ func shareLinkDownload(cmd *cobra.Command, args []string) error { arg.LinkPassword = opts.password.password } if target == "-" && commandOutputFormat(cmd) == output.FormatJSON { - return errors.New("`share-link download -` cannot be used with --output=json") + return invalidArgumentsError("`share-link download -` cannot be used with --output=json") } dbx := newSharedLinkClient(config) @@ -93,10 +93,10 @@ func shareLinkDownload(cmd *cobra.Command, args []string) error { if folder, ok := link.(*sharing.FolderLinkMetadata); ok { if !opts.recursive { - return errors.New("shared link is a folder (use --recursive to download folders)") + return invalidArgumentsError("shared link is a folder (use --recursive to download folders)") } if target == "-" { - return errors.New("cannot download shared-link folder to stdout") + return invalidArgumentsError("cannot download shared-link folder to stdout") } dst, err := sharedLinkFolderDownloadTarget(target, folder) @@ -112,7 +112,7 @@ func shareLinkDownload(cmd *cobra.Command, args []string) error { if target == "-" { if opts.recursive { - return errors.New("`share-link download -` cannot be used with --recursive") + return invalidArgumentsError("`share-link download -` cannot be used with --recursive") } if err := downloadSharedLinkToStdout(dbx, arg, cmd.OutOrStdout()); err != nil { return err @@ -150,20 +150,20 @@ func parseShareLinkDownloadOptions(cmd *cobra.Command) (shareLinkDownloadOptions return opts, err } if pathArg == "" { - return opts, errors.New("`--path` requires a non-empty path") + return opts, invalidArgumentsError("`--path` requires a non-empty path") } path, err := validatePath(pathArg) if err != nil { return opts, err } if path == "" { - return opts, errors.New("cannot download shared-link root with `--path`") + return opts, invalidArgumentsError("cannot download shared-link root with `--path`") } opts.path = path } if opts.path != "" && opts.recursive { - return opts, errors.New("`--path` cannot be used with --recursive") + return opts, invalidArgumentsError("`--path` cannot be used with --recursive") } return opts, nil diff --git a/cmd/share_link_download_test.go b/cmd/share_link_download_test.go index 0d28112..b1dd4a6 100644 --- a/cmd/share_link_download_test.go +++ b/cmd/share_link_download_test.go @@ -419,6 +419,9 @@ func TestShareLinkDownloadFolderRequiresRecursive(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "--recursive") { t.Fatalf("error = %v, want recursive error", err) } + if got, want := jsonErrorCode(err), jsonErrorCodeInvalidArguments; got != want { + t.Fatalf("jsonErrorCode = %q, want %q", got, want) + } if called { t.Fatal("GetSharedLinkFile should not be called") } @@ -440,6 +443,9 @@ func TestShareLinkDownloadFolderRejectsStdoutTarget(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "stdout") { t.Fatalf("error = %v, want stdout folder error", err) } + if got, want := jsonErrorCode(err), jsonErrorCodeInvalidArguments; got != want { + t.Fatalf("jsonErrorCode = %q, want %q", got, want) + } } func TestShareLinkDownloadFolderRecursiveDownloadsNestedFiles(t *testing.T) { diff --git a/cmd/share_link_info.go b/cmd/share_link_info.go index c760d12..e452469 100644 --- a/cmd/share_link_info.go +++ b/cmd/share_link_info.go @@ -39,12 +39,12 @@ type shareLinkInfoInput struct { func shareLinkInfo(cmd *cobra.Command, args []string) error { if len(args) != 1 { - return errors.New("`share-link info` requires a `url` argument") + return invalidArgumentsError("`share-link info` requires a `url` argument") } url := args[0] if url == "" { - return errors.New("`share-link info` requires a non-empty URL") + return invalidArgumentsError("`share-link info` requires a non-empty URL") } opts, err := parseShareLinkInfoOptions(cmd) @@ -93,7 +93,7 @@ func parseShareLinkInfoOptions(cmd *cobra.Command) (shareLinkInfoOptions, error) return opts, err } if path == "" { - return opts, errors.New("`--path` requires a non-empty path") + return opts, invalidArgumentsError("`--path` requires a non-empty path") } opts.path = path } diff --git a/cmd/share_link_json_test.go b/cmd/share_link_json_test.go index b8acb62..b9ae40f 100644 --- a/cmd/share_link_json_test.go +++ b/cmd/share_link_json_test.go @@ -133,6 +133,72 @@ func TestShareLinkListJSONOutputsResultsAndInput(t *testing.T) { } } +func TestDeprecatedShareListLinkJSONIncludesWarning(t *testing.T) { + stubSharedLinkClient(t, &mockSharedLinkClient{ + listSharedLinksFn: func(arg *sharing.ListSharedLinksArg) (*sharing.ListSharedLinksResult, error) { + return sharing.NewListSharedLinksResult([]sharing.IsSharedLinkMetadata{ + sharedLinkFile("/docs/report.txt", "https://example.com/report"), + }, false), nil + }, + }) + + var stdout bytes.Buffer + cmd := &cobra.Command{} + cmd.SetOut(&stdout) + setShareLinkOutputJSON(t, cmd) + + if err := shareListLinks(cmd, []string{"/docs/report.txt"}); err != nil { + t.Fatalf("shareListLinks error: %v", err) + } + + got := decodeShareLinkOperationOutputWithWarnings[shareLinkListInput, shareLinkJSONMetadata](t, stdout.Bytes()) + if len(got.Warnings) != 1 { + t.Fatalf("warnings = %+v, want one deprecation warning", got.Warnings) + } + warning := got.Warnings[0] + if warning.Code != jsonWarningCodeDeprecatedCommand { + t.Fatalf("warning code = %q, want %q", warning.Code, jsonWarningCodeDeprecatedCommand) + } + if !strings.Contains(warning.Message, "share-link list") { + t.Fatalf("warning message = %q, want share-link list", warning.Message) + } + if len(got.Results) != 1 || got.Results[0].Result.URL != "https://example.com/report" { + t.Fatalf("results = %#v, want listed shared link", got.Results) + } +} + +func TestDeprecatedShareListLinkJSONKeepsDeprecationTextOffStdout(t *testing.T) { + stubSharedLinkClient(t, &mockSharedLinkClient{ + listSharedLinksFn: func(arg *sharing.ListSharedLinksArg) (*sharing.ListSharedLinksResult, error) { + return sharing.NewListSharedLinksResult([]sharing.IsSharedLinkMetadata{ + sharedLinkFile("/docs/report.txt", "https://example.com/report"), + }, false), nil + }, + }) + + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd := &cobra.Command{} + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + setShareLinkOutputJSON(t, cmd) + + if err := shareListLinksCmd.RunE(cmd, []string{"/docs/report.txt"}); err != nil { + t.Fatalf("share list link error: %v", err) + } + + if strings.Contains(stdout.String(), `Command "link" is deprecated`) { + t.Fatalf("stdout = %q, want JSON only", stdout.String()) + } + got := decodeShareLinkOperationOutputWithWarnings[shareLinkListInput, shareLinkJSONMetadata](t, stdout.Bytes()) + if len(got.Warnings) != 1 || got.Warnings[0].Code != jsonWarningCodeDeprecatedCommand { + t.Fatalf("warnings = %+v, want deprecation warning", got.Warnings) + } + if stderr.String() != "" { + t.Fatalf("stderr = %q, RunE should not print Cobra deprecation text directly", stderr.String()) + } +} + func TestShareLinkInfoJSONOutputsPermissions(t *testing.T) { permissions := sharing.NewLinkPermissions(true, nil, true, true, true, true, true, false, false) permissions.ResolvedVisibility = &sharing.ResolvedVisibility{Tagged: dropbox.Tagged{Tag: sharing.ResolvedVisibilityPublic}} @@ -323,15 +389,21 @@ func decodeJSONOutput(t *testing.T, data []byte, out any) { } func decodeShareLinkOperationOutput[I, R any](t *testing.T, data []byte) shareLinkOperationOutputForTest[I, R] { + t.Helper() + got := decodeShareLinkOperationOutputWithWarnings[I, R](t, data) + if len(got.Warnings) != 0 { + t.Fatalf("warnings = %+v, want empty", got.Warnings) + } + return got +} + +func decodeShareLinkOperationOutputWithWarnings[I, R any](t *testing.T, data []byte) shareLinkOperationOutputForTest[I, R] { t.Helper() var got shareLinkOperationOutputForTest[I, R] decodeJSONOutput(t, data, &got) if got.Warnings == nil { t.Fatalf("warnings = nil, want empty array") } - if len(got.Warnings) != 0 { - t.Fatalf("warnings = %+v, want empty", got.Warnings) - } return got } diff --git a/cmd/share_link_password.go b/cmd/share_link_password.go index 5a6dd5e..32a86b0 100644 --- a/cmd/share_link_password.go +++ b/cmd/share_link_password.go @@ -16,7 +16,6 @@ package cmd import ( "bufio" - "errors" "fmt" "io" "os" @@ -66,7 +65,7 @@ func sharedLinkPasswordFromFlags(cmd *cobra.Command) (sharedLinkPasswordOptions, return sharedLinkPasswordOptions{}, nil } if sourceCount > 1 { - return sharedLinkPasswordOptions{}, errors.New("use only one of `--password`, `--password-prompt`, or `--password-file`") + return sharedLinkPasswordOptions{}, invalidArgumentsError("use only one of `--password`, `--password-prompt`, or `--password-file`") } var password string @@ -82,7 +81,7 @@ func sharedLinkPasswordFromFlags(cmd *cobra.Command) (sharedLinkPasswordOptions, return sharedLinkPasswordOptions{}, err } if password == "" { - return sharedLinkPasswordOptions{}, errors.New("shared link password cannot be empty") + return sharedLinkPasswordOptions{}, invalidArgumentsError("shared link password cannot be empty") } return sharedLinkPasswordOptions{ diff --git a/cmd/share_link_revoke.go b/cmd/share_link_revoke.go index 867c2ef..65b0994 100644 --- a/cmd/share_link_revoke.go +++ b/cmd/share_link_revoke.go @@ -51,12 +51,12 @@ func shareLinkRevoke(cmd *cobra.Command, args []string) error { } if len(args) != 1 { - return errors.New("`share-link revoke` requires a `url` argument") + return invalidArgumentsError("`share-link revoke` requires a `url` argument") } url := args[0] if url == "" { - return errors.New("`share-link revoke` requires a non-empty URL") + return invalidArgumentsError("`share-link revoke` requires a non-empty URL") } dbx := newSharedLinkClient(config) @@ -92,7 +92,7 @@ func parseShareLinkRevokeOptions(cmd *cobra.Command, args []string) (shareLinkRe return opts, nil } if len(args) != 0 { - return opts, errors.New("`--path` cannot be used with a shared link URL") + return opts, invalidArgumentsError("`--path` cannot be used with a shared link URL") } pathArg, err := localStringFlag(cmd, "path") @@ -100,7 +100,7 @@ func parseShareLinkRevokeOptions(cmd *cobra.Command, args []string) (shareLinkRe return opts, err } if pathArg == "" { - return opts, errors.New("`--path` requires a non-empty path") + return opts, invalidArgumentsError("`--path` requires a non-empty path") } path, err := validatePath(pathArg) @@ -108,7 +108,7 @@ func parseShareLinkRevokeOptions(cmd *cobra.Command, args []string) (shareLinkRe return opts, err } if path == "" { - return opts, errors.New("cannot revoke shared links for Dropbox root") + return opts, invalidArgumentsError("cannot revoke shared links for Dropbox root") } opts.path = path diff --git a/cmd/share_link_update.go b/cmd/share_link_update.go index bad9399..b71e3af 100644 --- a/cmd/share_link_update.go +++ b/cmd/share_link_update.go @@ -16,7 +16,6 @@ package cmd import ( "errors" - "fmt" "time" "github.com/dropbox/dbxcli/internal/output" @@ -47,12 +46,12 @@ type shareLinkUpdateInput struct { func shareLinkUpdate(cmd *cobra.Command, args []string) error { if len(args) != 1 { - return errors.New("`share-link update` requires a `url` argument") + return invalidArgumentsError("`share-link update` requires a `url` argument") } url := args[0] if url == "" { - return errors.New("`share-link update` requires a non-empty URL") + return invalidArgumentsError("`share-link update` requires a non-empty URL") } opts, err := parseShareLinkUpdateOptions(cmd) @@ -171,16 +170,16 @@ func parseShareLinkUpdateOptions(cmd *cobra.Command) (shareLinkUpdateOptions, er } if expiresChanged && removeExpiration { - return shareLinkUpdateOptions{}, errors.New("`--expires` and `--remove-expiration` cannot be used together") + return shareLinkUpdateOptions{}, invalidArgumentsError("`--expires` and `--remove-expiration` cannot be used together") } if allowDownload && disallowDownload { - return shareLinkUpdateOptions{}, errors.New("`--allow-download` and `--disallow-download` cannot be used together") + return shareLinkUpdateOptions{}, invalidArgumentsError("`--allow-download` and `--disallow-download` cannot be used together") } if password.set && removePassword { - return shareLinkUpdateOptions{}, errors.New("password-setting flags and `--remove-password` cannot be used together") + return shareLinkUpdateOptions{}, invalidArgumentsError("password-setting flags and `--remove-password` cannot be used together") } if !expiresChanged && !removeExpiration && !allowDownload && !disallowDownload && !audienceChanged && !password.set && !removePassword { - return shareLinkUpdateOptions{}, errors.New("at least one shared link setting flag is required") + return shareLinkUpdateOptions{}, invalidArgumentsError("at least one shared link setting flag is required") } var expires *time.Time @@ -191,7 +190,7 @@ func parseShareLinkUpdateOptions(cmd *cobra.Command) (shareLinkUpdateOptions, er } parsed, err := time.Parse(time.RFC3339, value) if err != nil { - return shareLinkUpdateOptions{}, fmt.Errorf("invalid --expires %q: use RFC3339 timestamp", value) + return shareLinkUpdateOptions{}, invalidArgumentsErrorf("invalid --expires %q: use RFC3339 timestamp", value) } expires = &parsed } diff --git a/cmd/team.go b/cmd/team.go index 3eb4901..52eda6d 100644 --- a/cmd/team.go +++ b/cmd/team.go @@ -14,11 +14,7 @@ package cmd -import ( - "fmt" - - "github.com/spf13/cobra" -) +import "github.com/spf13/cobra" // teamCmd represents the team command var teamCmd = &cobra.Command{ @@ -29,7 +25,7 @@ var teamCmd = &cobra.Command{ return err } if member, _ := cmd.Flags().GetString("as-member"); member != "" { - return fmt.Errorf("Flag `as-member` is invalid for team sub-commands") + return invalidArgumentsError("Flag `as-member` is invalid for team sub-commands") } return initDbx(cmd, args) },