Merge pull request #290 from dropbox/json-public-command-catalog

Publish command catalog, eliminate unknown types, and harden schema

Author: AndreyVMarkelov

README.md

@@ -170,7 +170,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. 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.
 
-Commands that intentionally do not support JSON output yet include `login`, `logout`, and `completion`. Cobra help output and shell-completion protocol commands are also text-only.
+Commands that intentionally do not support JSON output yet include `login`, `logout`, and `completion`. Cobra help output and shell-completion protocol commands are also text-only: `dbxcli --help --output=json`, `dbxcli --output=json` without a command, and command-specific help such as `dbxcli version --help --output=json` print text help.
 
 JSON error responses use stable `error.code` values:
 
@@ -187,12 +187,11 @@ JSON error responses use stable `error.code` values:
 | `rate_limited`                  | Dropbox rate limited the request.                                                 |
 | `dropbox_api_error`             | Dropbox returned an API error that does not map to a more specific code yet.      |
 | `structured_output_unsupported` | The command does not support `--output=json` yet.                                 |
-| `unsupported_output_format`     | `--output` was not `text` or `json`.                                              |
 | `unknown_command`               | Cobra could not resolve the command.                                              |
 | `unknown_flag`                  | Cobra could not resolve a flag.                                                   |
 | `command_failed`                | Fallback for failures without a more specific stable code.                        |
 
-Successful JSON responses for migrated commands return `ok: true`, `schema_version: "1"`, `command`, an `input` object, a `results` array, and a `warnings` array. Result payloads are command-specific. Public top-level schemas live under [docs/json-schema/v1](docs/json-schema/v1/). If a multi-target or recursive command fails after some side effects have already happened, dbxcli returns a JSON error envelope and does not include partial success results. For commands such as `mkdir`, each result reports what happened to the requested path:
+Successful JSON responses for migrated commands return `ok: true`, `schema_version: "1"`, `command`, an `input` object, a `results` array, and a `warnings` array. Result payloads are command-specific. Public top-level schemas and the command contract catalog live under [docs/json-schema/v1](docs/json-schema/v1/). If a multi-target or recursive command fails after some side effects have already happened, dbxcli returns a JSON error envelope and does not include partial success results. For commands such as `mkdir`, each result reports what happened to the requested path:
 
 ```json
 {

cmd/cp.go

@@ -18,6 +18,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/dropbox/dbxcli/internal/output"
 	"github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files"
 	"github.com/spf13/cobra"
 )
@@ -39,6 +40,7 @@ func cp(cmd *cobra.Command, args []string) error {
 	var cpErrors []error
 	var relocationArgs []*files.RelocationArg
 	var results []relocationResult
+	collectResults := commandOutputFormat(cmd) == output.FormatJSON
 
 	dbx := filesNewFunc(config)
 	destIsFolder := len(argsToCopy) > 1 || strings.HasSuffix(destination, "/") || isRemoteFolder(dbx, destination)
@@ -61,7 +63,15 @@ func cp(cmd *cobra.Command, args []string) error {
 			cpErrors = append(cpErrors, copyError)
 			continue
 		}
-		results = append(results, newRelocationResult(arg, res))
+		if collectResults {
+			result, err := newRelocationResult(arg, res)
+			if err != nil {
+				copyError := fmt.Errorf("copy %q to %q: %v", arg.FromPath, arg.ToPath, err)
+				cpErrors = append(cpErrors, copyError)
+				continue
+			}
+			results = append(results, result)
+		}
 	}
 
 	if len(cpErrors) > 0 {
@@ -71,6 +81,9 @@ func cp(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("cp: %d error(s)", len(cpErrors))
 	}
 
+	if !collectResults {
+		return nil
+	}
 	return renderJSONOperationOutput(cmd, nil, relocationOperationResults(relocationJSONStatusCopied, results))
 }
 

cmd/get.go

@@ -162,7 +162,7 @@ func getErrorOutput(opts getOptions) io.Writer {
 	return os.Stderr
 }
 
-func newGetResult(status, kind, source, target string, metadata files.IsMetadata) getResult {
+func newGetResult(status, kind, source, target string, metadata files.IsMetadata) (getResult, error) {
 	result := getResult{
 		Status: status,
 		Kind:   kind,
@@ -172,10 +172,13 @@ func newGetResult(status, kind, source, target string, metadata files.IsMetadata
 		},
 	}
 	if metadata != nil {
-		jsonResult := jsonMetadataFromDropbox(metadata)
+		jsonResult, err := jsonMetadataFromDropbox(metadata)
+		if err != nil {
+			return getResult{}, err
+		}
 		result.Result = &jsonResult
 	}
-	return result
+	return result, nil
 }
 
 func renderGetResults(cmd *cobra.Command, input getCommandInput, results []getResult) error {
@@ -352,7 +355,7 @@ func ensureLocalDirectoryResult(source, target string, metadata files.IsMetadata
 	if err := os.MkdirAll(target, 0755); err != nil {
 		return getResult{}, err
 	}
-	return newGetResult(status, getKindFolder, source, target, metadata), nil
+	return newGetResult(status, getKindFolder, source, target, metadata)
 }
 
 func relativeTo(base, full string) (string, error) {
@@ -376,7 +379,7 @@ func downloadFileWithResult(dbx files.Client, src string, dst string, opts getOp
 	if err != nil {
 		return getResult{}, err
 	}
-	return newGetResult(getStatusDownloaded, getKindFile, src, dst, metadata), nil
+	return newGetResult(getStatusDownloaded, getKindFile, src, dst, metadata)
 }
 
 func downloadFileWithMetadata(dbx files.Client, src string, dst string, errOut io.Writer) (*files.FileMetadata, error) {

cmd/json_contract_test.go

@@ -130,11 +130,13 @@ func TestStructuredOutputGoldenSuccessOutputAudit(t *testing.T) {
 			continue
 		}
 		assertGoldenJSONEqual(t, command, fixture, example)
+		assertGoldenSuccessOutputStatuses(t, command, fixture)
 	}
 	for command := range fixtures {
 		if !structuredSet[command] {
 			t.Errorf("golden success output includes non-structured command %q", command)
 		}
+		assertGoldenSuccessOutputStatuses(t, command, fixtures[command])
 	}
 	for command := range examples {
 		if !structuredSet[command] {
@@ -195,6 +197,18 @@ func TestPublicJSONSchemaFiles(t *testing.T) {
 	}
 }
 
+func TestPublicJSONCommandCatalogMatchesGoldenContract(t *testing.T) {
+	got := loadJSONContractFile(t, "../docs/json-schema/v1/commands.json", "public command catalog")
+	want := loadJSONGoldenContract(t)
+	if reflect.DeepEqual(got, want) {
+		return
+	}
+
+	gotJSON, _ := json.MarshalIndent(got, "", "  ")
+	wantJSON, _ := json.MarshalIndent(want, "", "  ")
+	t.Fatalf("public command catalog = %s, want %s", gotJSON, wantJSON)
+}
+
 func structuredOutputCommandPathsWithVersion() []string {
 	paths := structuredOutputCommandPaths(RootCmd)
 	return append(paths, NewVersionCommand("test").Name())
@@ -379,23 +393,29 @@ func jsonSuccessFixtureCoverage() map[string]jsonSuccessFixture {
 func loadJSONGoldenContract(t *testing.T) jsonGoldenContract {
 	t.Helper()
 
-	data, err := os.ReadFile("testdata/json_contract/success_schemas.json")
+	return loadJSONContractFile(t, "testdata/json_contract/success_schemas.json", "golden schema fixture")
+}
+
+func loadJSONContractFile(t *testing.T, file string, label string) jsonGoldenContract {
+	t.Helper()
+
+	data, err := os.ReadFile(file)
 	if err != nil {
-		t.Fatalf("read golden schema fixture: %v", err)
+		t.Fatalf("read %s %s: %v", label, file, err)
 	}
 
 	var raw struct {
 		Definitions map[string]json.RawMessage            `json:"definitions"`
 		Commands    map[string]map[string]json.RawMessage `json:"commands"`
 	}
 	if err := json.Unmarshal(data, &raw); err != nil {
-		t.Fatalf("decode raw golden schema fixture: %v", err)
+		t.Fatalf("decode raw %s %s: %v", label, file, err)
 	}
 	if len(raw.Definitions) == 0 {
-		t.Fatalf("golden schema fixture has no definitions")
+		t.Fatalf("%s %s has no definitions", label, file)
 	}
 	if len(raw.Commands) == 0 {
-		t.Fatalf("golden schema fixture has no commands")
+		t.Fatalf("%s %s has no commands", label, file)
 	}
 
 	requiredCommandFields := []string{
@@ -411,14 +431,14 @@ func loadJSONGoldenContract(t *testing.T) jsonGoldenContract {
 	for command, fields := range raw.Commands {
 		for _, field := range requiredCommandFields {
 			if _, ok := fields[field]; !ok {
-				t.Errorf("golden schema for %q missing %q", command, field)
+				t.Errorf("%s for %q missing %q", label, command, field)
 			}
 		}
 	}
 
 	var contract jsonGoldenContract
 	if err := json.Unmarshal(data, &contract); err != nil {
-		t.Fatalf("decode golden schema fixture: %v", err)
+		t.Fatalf("decode %s %s: %v", label, file, err)
 	}
 	return normalizeGoldenContract(contract)
 }
@@ -472,7 +492,6 @@ func expectedJSONErrorCodes() []string {
 		jsonErrorCodeStructuredOutputUnsupported,
 		jsonErrorCodeUnknownCommand,
 		jsonErrorCodeUnknownFlag,
-		jsonErrorCodeUnsupportedOutputFormat,
 	}
 }
 
@@ -501,6 +520,29 @@ func assertGoldenJSONEqual(t *testing.T, command string, fixture json.RawMessage
 	t.Errorf("golden output for %q = %s, want %s", command, gotJSON, wantJSON)
 }
 
+func assertGoldenSuccessOutputStatuses(t *testing.T, command string, fixture json.RawMessage) {
+	t.Helper()
+
+	var output jsonOperationOutput
+	if err := json.Unmarshal(fixture, &output); err != nil {
+		t.Fatalf("decode golden output for %q: %v", command, err)
+	}
+	for i, result := range output.Results {
+		if result.Status == "" {
+			t.Errorf("golden output for %q result %d has empty status", command, i)
+		}
+		if result.Status == "unknown" {
+			t.Errorf("golden output for %q result %d must not use unknown status", command, i)
+		}
+		if result.Kind == "" {
+			t.Errorf("golden output for %q result %d has empty kind", command, i)
+		}
+		if result.Kind == "unknown" {
+			t.Errorf("golden output for %q result %d must not use unknown kind", command, i)
+		}
+	}
+}
+
 func jsonGoldenSuccessOutputExamples() map[string]jsonOperationOutput {
 	file := sampleJSONFileMetadata("/Reports/old.pdf")
 	copyFile := sampleJSONFileMetadata("/Reports/copy.pdf")
@@ -789,7 +831,7 @@ func jsonCommandSchemas() map[string]jsonGoldenCommandSchema {
 		"mv":                operationSchema("empty", schemaRef("relocation_input"), "metadata", []string{relocationJSONStatusMoved}, metadataKinds(), nil),
 		"put":               operationSchema("put_input", schemaRef("put_result_input"), "metadata", []string{putStatusCreated, putStatusExisting, putStatusSkipped, putStatusUploaded}, []string{putKindFile, putKindFolder}, []string{jsonWarningCodeSkippedSymlink}),
 		"restore":           operationSchema("restore_input", schemaRef("restore_input"), "metadata", []string{restoreStatusRestored}, []string{restoreKindFile}, nil),
-		"revs":              operationSchema("revs_input", schemaRef("empty"), "metadata", []string{revsJSONStatusRevision}, []string{"file", "unknown"}, nil),
+		"revs":              operationSchema("revs_input", schemaRef("empty"), "metadata", []string{revsJSONStatusRevision}, []string{"file"}, nil),
 		"rm":                operationSchema("empty", schemaRef("remove_input"), "metadata", []string{removeJSONStatusDeleted, removeJSONStatusPermanentlyDeleted}, metadataKinds(), nil),
 		"search":            operationSchema("search_input", schemaRef("empty"), "metadata", []string{searchJSONStatusFound}, metadataKinds(), nil),
 		"share list folder": operationSchema("empty", schemaRef("empty"), "share_folder", []string{shareFolderJSONStatusListed}, []string{shareFolderJSONKindFolder}, nil),
@@ -834,7 +876,7 @@ func schemaRef(name string) *string {
 }
 
 func metadataKinds() []string {
-	return []string{"deleted", "file", "folder", "unknown"}
+	return []string{"deleted", "file", "folder"}
 }
 
 func shareLinkKinds() []string {
@@ -933,6 +975,11 @@ func assertGoldenCommandStatuses(t *testing.T, command string, schema jsonGolden
 			t.Errorf("golden schema for %q must not allow unknown result status", command)
 		}
 	}
+	for _, kind := range schema.Kinds {
+		if kind == "unknown" {
+			t.Errorf("golden schema for %q must not allow unknown result kind", command)
+		}
+	}
 }
 
 func normalizeGoldenContract(contract jsonGoldenContract) jsonGoldenContract {

cmd/json_metadata.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files"
@@ -18,11 +19,11 @@ type jsonMetadata struct {
 	Deleted        bool    `json:"deleted,omitempty"`
 }
 
-func jsonMetadataFromDropbox(metadata files.IsMetadata) jsonMetadata {
+func jsonMetadataFromDropbox(metadata files.IsMetadata) (jsonMetadata, error) {
 	switch m := metadata.(type) {
 	case *files.FileMetadata:
 		if m == nil {
-			return jsonMetadata{Type: "unknown"}
+			return jsonMetadata{}, fmt.Errorf("unexpected nil Dropbox file metadata")
 		}
 		size := m.Size
 		return jsonMetadata{
@@ -34,38 +35,42 @@ func jsonMetadataFromDropbox(metadata files.IsMetadata) jsonMetadata {
 			Size:           &size,
 			ServerModified: jsonTime(m.ServerModified),
 			ClientModified: jsonTime(m.ClientModified),
-		}
+		}, nil
 	case *files.FolderMetadata:
 		if m == nil {
-			return jsonMetadata{Type: "unknown"}
+			return jsonMetadata{}, fmt.Errorf("unexpected nil Dropbox folder metadata")
 		}
 		return jsonMetadata{
 			Type:        "folder",
 			PathDisplay: m.PathDisplay,
 			PathLower:   m.PathLower,
 			ID:          m.Id,
-		}
+		}, nil
 	case *files.DeletedMetadata:
 		if m == nil {
-			return jsonMetadata{Type: "unknown"}
+			return jsonMetadata{}, fmt.Errorf("unexpected nil Dropbox deleted metadata")
 		}
 		return jsonMetadata{
 			Type:        "deleted",
 			PathDisplay: m.PathDisplay,
 			PathLower:   m.PathLower,
 			Deleted:     true,
-		}
+		}, nil
 	default:
-		return jsonMetadata{Type: "unknown"}
+		return jsonMetadata{}, fmt.Errorf("unexpected Dropbox metadata type %T", metadata)
 	}
 }
 
-func jsonMetadataListFromDropbox(entries []files.IsMetadata) []jsonMetadata {
+func jsonMetadataListFromDropbox(entries []files.IsMetadata) ([]jsonMetadata, error) {
 	result := make([]jsonMetadata, 0, len(entries))
 	for _, entry := range entries {
-		result = append(result, jsonMetadataFromDropbox(entry))
+		metadata, err := jsonMetadataFromDropbox(entry)
+		if err != nil {
+			return nil, err
+		}
+		result = append(result, metadata)
 	}
-	return result
+	return result, nil
 }
 
 func jsonTime(t time.Time) *string {

cmd/json_metadata_test.go

@@ -24,7 +24,10 @@ func TestJSONMetadataFromDropboxFile(t *testing.T) {
 		ServerModified: serverModified,
 	}
 
-	got := jsonMetadataFromDropbox(metadata)
+	got, err := jsonMetadataFromDropbox(metadata)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if got.Type != "file" {
 		t.Fatalf("Type = %q, want file", got.Type)
@@ -63,7 +66,10 @@ func TestJSONMetadataFromDropboxFolder(t *testing.T) {
 		Id: "id:folder",
 	}
 
-	got := jsonMetadataFromDropbox(metadata)
+	got, err := jsonMetadataFromDropbox(metadata)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if got.Type != "folder" {
 		t.Fatalf("Type = %q, want folder", got.Type)
@@ -84,7 +90,10 @@ func TestJSONMetadataFromDropboxDeleted(t *testing.T) {
 		},
 	}
 
-	got := jsonMetadataFromDropbox(metadata)
+	got, err := jsonMetadataFromDropbox(metadata)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if got.Type != "deleted" {
 		t.Fatalf("Type = %q, want deleted", got.Type)
@@ -93,3 +102,9 @@ func TestJSONMetadataFromDropboxDeleted(t *testing.T) {
 		t.Fatal("Deleted = false, want true")
 	}
 }
+
+func TestJSONMetadataFromDropboxRejectsUnknownMetadata(t *testing.T) {
+	if _, err := jsonMetadataFromDropbox(nil); err == nil {
+		t.Fatal("expected nil metadata to fail")
+	}
+}

cmd/json_output.go

@@ -122,12 +122,6 @@ func normalizeJSONOperationResults(results []jsonOperationResult) []jsonOperatio
 }
 
 func normalizeJSONOperationResult(result jsonOperationResult) jsonOperationResult {
-	if result.Status == "" {
-		result.Status = "unknown"
-	}
-	if result.Kind == "" {
-		result.Kind = "unknown"
-	}
 	result.Input = normalizeJSONObject(result.Input)
 	result.Result = normalizeJSONObject(result.Result)
 	return result