diff --git a/README.md b/README.md index 733bcd2..54d5e7a 100644 --- a/README.md +++ b/README.md @@ -113,10 +113,19 @@ accepted. kaiten customers list kaiten licenses get team-v1 --output yaml kaiten licenses entitlements list team-v1 -kaiten instances audit-trails acme-production --event-name instance.updated --after 2026-01-01T00:00:00Z --limit 100 +kaiten instances audit-trails acme-production --event-name INSTANCE_UPDATED --after 2026-01-01T00:00:00Z --limit 100 ``` -`--output` (or `KAITEN_OUTPUT`) selects `table` (the default), `json` or `yaml`. +`--output` (or `KAITEN_OUTPUT`) selects `table`, `json` or `yaml`. + +Without it, the format follows what the command returns. A list prints a table. A single +resource (`get`, `create`, a usage report, a new token) prints YAML, and so does +`--output table` there, a single resource having no columns. A command the API answers +with no body (`delete`, most `update`s) prints one line, or a document with a `message` +key under `--output json` and `--output yaml`. + +JSON and YAML output carry the same document: the API's field names and values, in the +same order. ### Writing @@ -137,6 +146,26 @@ kaiten components create --payload '{"name": "api", "version": "1.4.0"}' Feature flags carry targeting rules and variants, so `feature-flags create` and `update` take `--file` or `--payload` only. +A file or a payload uses the API's field names, in JSON or in YAML, and both make the +same request: + +```yaml +# customer.yaml +name: Acme Corp +externalCustomerId: crm-4711 +``` + +A YAML document is read as the JSON it stands for. A value written bare for a text field +stays text (`version: 1.0` is the version `1.0`), and a YAML key the request has no field +for is an error that names the key, before anything is sent. JSON skips such a key +instead, so a resource printed with `--output json` can be edited and written back with +the fields only the API sets (`id`, `createdAt`, ...) still in it; printed as YAML, it has +to lose them first. + +With inline flags, `instances update` keeps what its optional flags leave out: the +deployment zone, the slug, and the metadata, which the command reads from the instance and +sends back unless `--metadata-json` gives another (`--metadata-json '{}'` clears it). + ### Licenses and entitlements ```shell @@ -154,11 +183,21 @@ kaiten licenses entitlements update team-v1 monthly-orders --number 1500 ``` The API replaces an entitlement or a grant with what it is sent, resetting what the request -leaves out. So `entitlements update` and `licenses entitlements update` read the resource -first when given inline flags, and send back everything the flags do not change: a -periodic entitlement keeps its window, which the API refuses to see dropped, and a grant -keeps its allowance unless `--overage-percent` says otherwise. `--file` and `--payload` are -sent as written. +leaves out. So both update commands read the resource first and send back what the +invocation does not change, each in its own way: + +- `entitlements update` does it with inline flags: every field the flags do not name is + sent back as it was read, so a periodic entitlement keeps its window, which the API + refuses to see dropped. `--file` and `--payload` are sent as written there, and carry + every field the entitlement keeps. +- `licenses entitlements update` does it whenever `--overage-percent` is absent, whatever + carries the value: `--number`, `--boolean`, an object, or `--file` and `--payload`, which + hold the value and nothing else. The grant keeps its allowance when the new value is a + finite number. `--overage-percent` sets another, and the grant is then not read. + +The API numbers the versions of a license itself. `licenses create` has no `--version`, +and on `licenses update` the flag does not set the version: it states the one the license +is expected to have, and the API refuses the update when the license has another. ### Usage metering @@ -191,6 +230,9 @@ prompting, so scripts and CI pipelines pass `--yes`: kaiten customers delete acme --yes ``` +Without `--yes` (`-y`), a run that cannot prompt changes nothing and exits with code `2`. +At the prompt, any answer but `y` or `yes` cancels the command, which exits with code `1`. + ## Exit codes Every command reports the kind of failure through its exit code, so a script can diff --git a/internal/cmd/common_output_test.go b/internal/cmd/common_output_test.go index ed1f6f9..bdb59fe 100644 --- a/internal/cmd/common_output_test.go +++ b/internal/cmd/common_output_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "reflect" "strings" "testing" @@ -122,6 +123,93 @@ func TestWriteMessageFallsBackToABareLineForNonStructuredFormats(t *testing.T) { } } +// TestYAMLOutputCarriesTheDocumentJSONOutputCarries pins YAML, the default output of every +// command that returns one resource, to the document --output json prints. The API's types +// describe themselves in JSON only, so YAML used to name their fields after the Go ones +// (externalcustomerid) and to print a typed value -- a grant's, a usage counter's -- as an +// empty mapping. +func TestYAMLOutputCarriesTheDocumentJSONOutputCarries(t *testing.T) { + const usagePath = "/instances/sakura-tokyo-prod/entitlements/monthly-orders/usage" + api := givenRoutes(t, map[string]route{ + "GET /customers/sakura-tokyo": {status: http.StatusOK, body: `{ + "id": "cus_1", "name": "Sakura Tokyo", "slug": "sakura-tokyo", "externalCustomerId": "crm-1024", + "createdAt": "2026-10-01T09:00:00Z", "createdBy": {"id": "usr_1", "name": "Aiko"}}`}, + "GET /licenses/premium-v2/entitlements/monthly-orders": {status: http.StatusOK, body: `{ + "entitlementSlug": "monthly-orders", "entitlementType": "NUMBER", + "value": {"type": "number", "value": 1000}, "limitCapExceededOveragePercent": 20}`}, + "GET " + usagePath: {status: http.StatusOK, body: `{ + "entitlementId": "ent_1", "entitlementSlug": "monthly-orders", + "licenseId": "lic_1", "licenseSlug": "premium-v2", + "value": {"type": "number", "value": 730.5}, "limit": {"type": "number", "value": 1000}}`}, + }) + + tests := []struct { + name string + args []string + want string + }{ + { + name: "a resource keeps the API's field names", + args: []string{"customers", "get", "sakura-tokyo"}, + want: "externalCustomerId: crm-1024", + }, + { + name: "a grant keeps its typed value", + args: []string{"licenses", "entitlements", "get", "premium-v2", "monthly-orders"}, + want: "limitCapExceededOveragePercent: 20", + }, + { + name: "a usage counter keeps its value and its limit", + args: []string{"instances", "usage", "get", "sakura-tokyo-prod", "monthly-orders"}, + want: "value: 730.5", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + givenIsolatedConfig(t) + run := func(extra ...string) string { + return runSuccessfully(t, append(append([]string{"--base-url", api.URL}, extra...), test.args...)...) + } + + var want any + if err := json.Unmarshal([]byte(run("--output", "json")), &want); err != nil { + t.Fatalf("decode json output error = %v", err) + } + + // YAML is what the command prints when no format is selected, too. + for _, selected := range [][]string{nil, {"--output", "yaml"}} { + stdout := run(selected...) + expectOutputContains(t, stdout, test.want) + + var decoded any + if err := yaml.Unmarshal([]byte(stdout), &decoded); err != nil { + t.Fatalf("decode yaml output error = %v, stdout = %q", err, stdout) + } + if got := throughJSON(t, decoded); !reflect.DeepEqual(got, want) { + t.Fatalf("yaml output = %v, want the json document %v", got, want) + } + } + }) + } +} + +// throughJSON re-reads a decoded YAML document the way JSON types it, so that it compares +// with a decoded JSON document: every number a float64. +func throughJSON(t *testing.T, value any) any { + t.Helper() + + raw, err := json.Marshal(value) + if err != nil { + t.Fatalf("encode decoded yaml error = %v", err) + } + var out any + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatalf("decode re-encoded yaml error = %v", err) + } + return out +} + // givenCustomerAPI serves the customer and token endpoints the rendering tests exercise. func givenCustomerAPI(t *testing.T) *httptest.Server { t.Helper() diff --git a/internal/cmd/components.go b/internal/cmd/components.go index c63510b..45332ad 100644 --- a/internal/cmd/components.go +++ b/internal/cmd/components.go @@ -102,6 +102,7 @@ func newComponentsCreateCommand() *cobra.Command { } addInputSourceFlags(cmd, &file, &payload, "component") inline.register(cmd) + inline.registerPreviousComponentID(cmd) return cmd } @@ -173,10 +174,17 @@ func (f *componentInputFlags) register(cmd *cobra.Command) { cmd.Flags().StringVar(&f.name, "name", "", "Component name") cmd.Flags().StringVar(&f.version, "version", "", "Component version") cmd.Flags().StringVar(&f.description, "description", "", "Component description") - cmd.Flags().StringVar(&f.previousComponentID, "previous-component-id", "", "Previous component ID") cmd.Flags().StringVar(&f.slug, "slug", "", "Component slug") } +// registerPreviousComponentID adds --previous-component-id to create only. The link to the +// version a component follows is set once, when it is created: the API's update refuses +// the field (422 UpdateComponent.PreviousComponentNotSettable) and the request the SDK +// builds there does not carry it, so on update the flag was accepted and dropped. +func (f *componentInputFlags) registerPreviousComponentID(cmd *cobra.Command) { + cmd.Flags().StringVar(&f.previousComponentID, "previous-component-id", "", "ID of the component this one is the next version of") +} + func (f *componentInputFlags) changed(cmd *cobra.Command) bool { return anyFlagChanged(cmd, "name", "version", "description", "previous-component-id", "slug") } diff --git a/internal/cmd/components_test.go b/internal/cmd/components_test.go new file mode 100644 index 0000000..8ab9a02 --- /dev/null +++ b/internal/cmd/components_test.go @@ -0,0 +1,44 @@ +package cmd + +import ( + "net/http" + "strings" + "testing" +) + +// TestPreviousComponentIDIsOfferedOnCreateOnly pins --previous-component-id to the command +// where it does something. The link to the version a component follows is set once, at +// creation: the API's update refuses the field, and the request built there never carried +// it, so `components update --previous-component-id` printed the component and exited 0 +// with the link untouched. Create still sends it; update no longer takes the flag. +func TestPreviousComponentIDIsOfferedOnCreateOnly(t *testing.T) { + const component = `{"id": "cmp_2", "name": "kitchen-display", "slug": "kitchen-display-1-3-0", "version": "1.3.0"}` + api := givenRoutes(t, map[string]route{ + "POST /components": {status: http.StatusCreated, body: component}, + "PUT /components/kitchen-display-1-3-0": {status: http.StatusOK, body: component}, + }) + givenIsolatedConfig(t) + + runSuccessfully(t, "--base-url", api.URL, "components", "create", + "--name", "kitchen-display", "--version", "1.3.0", "--previous-component-id", "cmp_1") + runSuccessfully(t, "--base-url", api.URL, "components", "update", "kitchen-display-1-3-0", + "--name", "kitchen-display", "--version", "1.3.1") + + calls := api.calls() + expectCalls(t, calls, "POST /components", "PUT /components/kitchen-display-1-3-0") + expectBody(t, calls[0].body, map[string]any{ + "name": "kitchen-display", "version": "1.3.0", "previousComponentId": "cmp_1", + }) + expectBody(t, calls[1].body, map[string]any{"name": "kitchen-display", "version": "1.3.1"}) + + code, err := executeForExitCode(t, "--base-url", api.URL, "components", "update", "kitchen-display-1-3-0", + "--name", "kitchen-display", "--version", "1.3.1", "--previous-component-id", "cmp_1") + + if err == nil || !strings.Contains(err.Error(), "unknown flag: --previous-component-id") { + t.Fatalf("Execute() error = %v, want --previous-component-id refused as an unknown flag", err) + } + expectExitCode(t, code, exitUsage) + if got := len(api.calls()); got != len(calls) { + t.Fatalf("the API was called %d more times, want the CLI to stop before the request", got-len(calls)) + } +} diff --git a/internal/cmd/entitlements_test.go b/internal/cmd/entitlements_test.go index 932b2c3..90eb89b 100644 --- a/internal/cmd/entitlements_test.go +++ b/internal/cmd/entitlements_test.go @@ -5,6 +5,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "reflect" "sync" "testing" @@ -137,7 +138,7 @@ type route struct { } // routedAPI is a fake API that answers each "METHOD path" with its route and records every -// call, in order, with its decoded body. +// call, in order, with its query and its decoded body. type routedAPI struct { *httptest.Server @@ -146,8 +147,9 @@ type routedAPI struct { } type recordedCall struct { - call string - body map[string]any + call string + query url.Values + body map[string]any } func (a *routedAPI) calls() []recordedCall { @@ -170,7 +172,7 @@ func givenRoutes(t *testing.T, routes map[string]route) *routedAPI { } } api.mu.Lock() - api.recorded = append(api.recorded, recordedCall{call: call, body: body}) + api.recorded = append(api.recorded, recordedCall{call: call, query: r.URL.Query(), body: body}) api.mu.Unlock() answer, ok := routes[call] diff --git a/internal/cmd/input_sources_test.go b/internal/cmd/input_sources_test.go index a2e581a..669444c 100644 --- a/internal/cmd/input_sources_test.go +++ b/internal/cmd/input_sources_test.go @@ -59,6 +59,127 @@ func TestInputSourcesBuildTheRequestBody(t *testing.T) { } } +// TestYAMLInputSendsTheRequestItsJSONSends pins a YAML --file or --payload to the request +// the same document makes in JSON. The fields of a request are named for JSON only, so YAML +// used to fill the ones of a single word and lose the rest without an error: a customer +// went out without its externalCustomerId, a feature flag with a null default_variant, and +// a grant with no value at all. +func TestYAMLInputSendsTheRequestItsJSONSends(t *testing.T) { + tests := []struct { + name string + args []string + route string + yaml string + json string + want map[string]any + }{ + { + name: "a key of several words", + args: []string{"customers", "create"}, + route: "POST /customers", + yaml: "name: Sakura Tokyo\nexternalCustomerId: crm-1024\n", + json: `{"name": "Sakura Tokyo", "externalCustomerId": "crm-1024"}`, + want: map[string]any{"name": "Sakura Tokyo", "externalCustomerId": "crm-1024"}, + }, + { + name: "a version that reads as a number stays the text it is", + args: []string{"components", "create"}, + route: "POST /components", + yaml: "name: api\nversion: 1.0\npreviousComponentId: cmp_0\n", + json: `{"name": "api", "version": "1.0", "previousComponentId": "cmp_0"}`, + want: map[string]any{"name": "api", "version": "1.0", "previousComponentId": "cmp_0"}, + }, + { + name: "dates and metadata", + args: []string{"instances", "create"}, + route: "POST /instances", + yaml: "name: Sakura Tokyo Prod\ndescription: Production\ncustomerId: cus_1\nlicenseId: lic_1\n" + + "startLicenseDate: 2026-01-01T00:00:00Z\nendLicenseDate: 2027-01-01T00:00:00Z\n" + + "metadata:\n region: kanto\n seats: 40\n", + json: `{"name": "Sakura Tokyo Prod", "description": "Production", "customerId": "cus_1", "licenseId": "lic_1", + "startLicenseDate": "2026-01-01T00:00:00Z", "endLicenseDate": "2027-01-01T00:00:00Z", + "metadata": {"region": "kanto", "seats": 40}}`, + want: map[string]any{ + "name": "Sakura Tokyo Prod", "description": "Production", "customerId": "cus_1", "licenseId": "lic_1", + "startLicenseDate": "2026-01-01T00:00:00Z", "endLicenseDate": "2027-01-01T00:00:00Z", + "metadata": map[string]any{"region": "kanto", "seats": float64(40)}, + }, + }, + { + name: "a grant's typed value", + args: []string{"licenses", "entitlements", "associate", "standard", "monthly-orders"}, + route: "POST /licenses/standard/entitlements", + yaml: "type: number\nvalue: 2000\n", + json: `{"type": "number", "value": 2000}`, + want: map[string]any{"entitlementSlug": "monthly-orders", "value": numberGrant(2000)}, + }, + { + name: "a feature flag's snake_case keys", + args: []string{"feature-flags", "create"}, + route: "POST /feature-flags", + yaml: "name: Delivery tracking\ntype: boolean\nenabled: true\nevent_name: delivery-tracking-enabled\n" + + "default_variant:\n key: \"off\"\n value: false\n", + json: `{"name": "Delivery tracking", "type": "boolean", "enabled": true, "event_name": "delivery-tracking-enabled", + "default_variant": {"key": "off", "value": false}}`, + want: map[string]any{ + "name": "Delivery tracking", "type": "boolean", "enabled": true, "event_name": "delivery-tracking-enabled", + "default_variant": map[string]any{"key": "off", "value": false}, + "description": nil, "metadata": nil, "targetings": nil, "variants": nil, + }, + }, + } + + for _, test := range tests { + sources := []struct { + name string + args func(t *testing.T) []string + }{ + {name: "a json file", args: func(t *testing.T) []string { return []string{"--file", givenInputFile(t, "input.json", test.json)} }}, + {name: "a yaml file", args: func(t *testing.T) []string { return []string{"--file", givenInputFile(t, "input.yaml", test.yaml)} }}, + {name: "a yaml payload", args: func(*testing.T) []string { return []string{"--payload", test.yaml} }}, + } + for _, source := range sources { + t.Run(test.name+"/"+source.name, func(t *testing.T) { + // Every create answers with a body the command only prints: an empty object will do. + api := givenRoutes(t, map[string]route{test.route: {status: http.StatusCreated, body: `{}`}}) + givenIsolatedConfig(t) + + args := append([]string{"--base-url", api.URL}, test.args...) + runSuccessfully(t, append(args, source.args(t)...)...) + + expectCalls(t, api.calls(), test.route) + expectBody(t, api.calls()[0].body, test.want) + }) + } + } +} + +// TestYAMLInputRefusesAKeyTheRequestHasNoFieldFor is the other half: a key that reaches no +// field stops the command before the request, naming the key, instead of being left out of +// a request that then succeeds. +func TestYAMLInputRefusesAKeyTheRequestHasNoFieldFor(t *testing.T) { + const document = "name: Sakura Tokyo\nexternal_customer_id: crm-1024\n" + + for name, source := range map[string]func(t *testing.T) []string{ + "a yaml file": func(t *testing.T) []string { return []string{"--file", givenInputFile(t, "customer.yaml", document)} }, + "a yaml payload": func(*testing.T) []string { return []string{"--payload", document} }, + } { + t.Run(name, func(t *testing.T) { + api := givenRoutes(t, map[string]route{}) + + args := append([]string{"--base-url", api.URL, "customers", "create"}, source(t)...) + _, err := executeForExitCode(t, args...) + + if err == nil || !strings.Contains(err.Error(), `unknown field "external_customer_id"`) { + t.Fatalf("Execute() error = %v, want it to name the unknown key", err) + } + if calls := api.calls(); len(calls) != 0 { + t.Fatalf("the API was called %d times, want the CLI to stop before the request", len(calls)) + } + }) + } +} + // TestOptionalFieldsAreOmittedWhenTheirFlagIsUnset pins the rule optionalString exists for: // a field the caller never mentioned must not reach the API at all, because an empty string // would overwrite whatever the record already holds. diff --git a/internal/cmd/instances.go b/internal/cmd/instances.go index 5847d74..ec15a9f 100644 --- a/internal/cmd/instances.go +++ b/internal/cmd/instances.go @@ -29,7 +29,6 @@ func newInstancesCommand() *cobra.Command { } func newInstancesListCommand() *cobra.Command { - var includeDeleted bool cmd := &cobra.Command{ Use: "list", Short: "List instances", @@ -41,7 +40,7 @@ func newInstancesListCommand() *cobra.Command { } ctx, cancel := commandContext(cmd) defer cancel() - items, err := client.Instances.List(ctx, &sdk.InstancesListOptions{IncludeDeleted: includeDeleted}) + items, err := client.Instances.List(ctx, nil) if err != nil { return err } @@ -60,7 +59,6 @@ func newInstancesListCommand() *cobra.Command { }) }, } - cmd.Flags().BoolVar(&includeDeleted, "include-deleted", false, "Include soft-deleted instances") return cmd } @@ -121,7 +119,13 @@ func newInstancesUpdateCommand() *cobra.Command { cmd := &cobra.Command{ Use: "update ", Short: "Update an instance from --file, --payload, or inline flags", - Args: cobra.ExactArgs(1), + Long: "Update an instance from --file, --payload, or inline flags. The API replaces the\n" + + "instance with what it is sent, so inline flags name every required field again. What\n" + + "the optional flags leave out is kept: the deployment zone and the slug by the API, and\n" + + "the metadata by the command, which reads the instance first and sends its metadata\n" + + "back unless --metadata-json gives another -- '{}' to clear it. --file and --payload\n" + + "are sent as written.", + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { inputValue, err := resolveInput(file, payload, inline.changed(cmd), inline.build(cmd)) if err != nil { @@ -133,6 +137,18 @@ func newInstancesUpdateCommand() *cobra.Command { } ctx, cancel := commandContext(cmd) defer cancel() + // The API replaces the metadata with what it is sent and the request cannot + // leave it out, so flags that name none would wipe it with the empty object + // they build. They carry back what the instance holds instead. + if inline.changed(cmd) && !flagChanged(cmd, "metadata-json") { + current, err := client.Instances.Get(ctx, args[0]) + if err != nil { + return err + } + if current.Metadata != nil { + inputValue.Metadata = *current.Metadata + } + } if err := client.Instances.Update(ctx, args[0], inputValue); err != nil { return err } @@ -174,7 +190,6 @@ func newInstancesAuditTrailsCommand() *cobra.Command { var after string var before string var limit int32 - var offset int32 cmd := &cobra.Command{ Use: "audit-trails ", @@ -201,7 +216,6 @@ func newInstancesAuditTrailsCommand() *cobra.Command { After: afterTime, Before: beforeTime, Limit: optionalInt32(limit), - Offset: optionalInt32(offset), }) if err != nil { return err @@ -226,7 +240,6 @@ func newInstancesAuditTrailsCommand() *cobra.Command { cmd.Flags().StringVar(&after, "after", "", "Only include events after this RFC3339 timestamp") cmd.Flags().StringVar(&before, "before", "", "Only include events before this RFC3339 timestamp") cmd.Flags().Int32Var(&limit, "limit", 0, "Limit results") - cmd.Flags().Int32Var(&offset, "offset", 0, "Offset results") return cmd } @@ -373,7 +386,7 @@ func loadUsageMetadata(metadataFile, metadataPayload string) (map[string]any, er } // optionalInt32 omits a paging bound the caller left at its zero value: the API applies its -// own default for an absent limit or offset, which sending 0 would override. +// own default for an absent limit, which sending 0 would override. func optionalInt32(value int32) *int32 { if value == 0 { return nil diff --git a/internal/cmd/instances_test.go b/internal/cmd/instances_test.go new file mode 100644 index 0000000..500568b --- /dev/null +++ b/internal/cmd/instances_test.go @@ -0,0 +1,175 @@ +package cmd + +import ( + "net/http" + "strings" + "testing" +) + +const emptyPage = `{"items": [], "nextCursor": null, "hasMore": false}` + +// TestInstancesReadsOfferOnlyTheFiltersTheAPIReads pins `instances list` and `instances +// audit-trails` to the query parameters the API declares. Two flags were accepted and +// changed nothing: --include-deleted was sent as include_deleted, which the list takes no +// such parameter for and ignores, and --offset was not sent at all, the audit trail being +// paged by cursor. Neither is a flag any more, and neither parameter is sent. +func TestInstancesReadsOfferOnlyTheFiltersTheAPIReads(t *testing.T) { + const auditTrails = "GET /instances/sakura-tokyo-prod/audit-trails" + api := givenRoutes(t, map[string]route{ + "GET /instances": {status: http.StatusOK, body: emptyPage}, + auditTrails: {status: http.StatusOK, body: emptyPage}, + }) + givenIsolatedConfig(t) + + runSuccessfully(t, "--base-url", api.URL, "instances", "list") + runSuccessfully(t, "--base-url", api.URL, "instances", "audit-trails", "sakura-tokyo-prod", + "--event-name", "INSTANCE_UPDATED", "--after", "2026-10-01T00:00:00Z", "--limit", "10") + + calls := api.calls() + expectCalls(t, calls, "GET /instances", auditTrails) + for _, call := range calls { + for _, parameter := range []string{"include_deleted", "offset"} { + if call.query.Has(parameter) { + t.Errorf("%s was sent %s=%q, which the API does not read", call.call, parameter, call.query.Get(parameter)) + } + } + } + // The filters the API does read still go out. + for parameter, want := range map[string]string{ + "event_name": "INSTANCE_UPDATED", "after": "2026-10-01T00:00:00Z", "limit": "10", + } { + if got := calls[1].query.Get(parameter); got != want { + t.Errorf("%s was sent %s=%q, want %q", calls[1].call, parameter, got, want) + } + } + + for _, refused := range []struct { + flag string + args []string + }{ + {flag: "--include-deleted", args: []string{"instances", "list", "--include-deleted"}}, + {flag: "--offset", args: []string{"instances", "audit-trails", "sakura-tokyo-prod", "--offset", "5"}}, + } { + code, err := executeForExitCode(t, append([]string{"--base-url", api.URL}, refused.args...)...) + + if err == nil || !strings.Contains(err.Error(), "unknown flag: "+refused.flag) { + t.Fatalf("Execute(%v) error = %v, want %s refused as an unknown flag", refused.args, err, refused.flag) + } + expectExitCode(t, code, exitUsage) + } + if got := len(api.calls()); got != len(calls) { + t.Fatalf("the API was called %d more times, want the CLI to stop before the request", got-len(calls)) + } +} + +// TestInstancesUpdateKeepsTheMetadataItDoesNotName pins what an inline update does with an +// instance's metadata. The API replaces the metadata with what it is sent, and the request +// cannot leave it out, so an update whose flags named none used to send an empty object and +// wipe it. The command now reads the instance first and sends its metadata back, unless +// --metadata-json gives the metadata to store -- an empty object included, which is how it +// is cleared on purpose. --file and --payload are still sent as written. +func TestInstancesUpdateKeepsTheMetadataItDoesNotName(t *testing.T) { + const withMetadata = `{ + "id": "ins_1", "name": "Sakura Tokyo Prod", "slug": "sakura-tokyo-prod", "description": "Production", + "customerId": "cus_1", "licenseId": "lic_1", + "startLicenseDate": "2026-01-01T00:00:00Z", "endLicenseDate": "2027-01-01T00:00:00Z", + "metadata": {"region": "kanto", "seats": 40}}` + const withoutMetadata = `{ + "id": "ins_1", "name": "Sakura Tokyo Prod", "slug": "sakura-tokyo-prod", "description": "Production", + "customerId": "cus_1", "licenseId": "lic_1", + "startLicenseDate": "2026-01-01T00:00:00Z", "endLicenseDate": "2027-01-01T00:00:00Z"}` + inline := []string{ + "--name", "Sakura Tokyo Prod", "--description", "Production, Shibuya", "--customer-id", "cus_1", "--license-id", "lic_1", + "--start-license-date", "2026-01-01T00:00:00Z", "--end-license-date", "2027-01-01T00:00:00Z", + } + + tests := []struct { + name string + current string + args []string + wantCalls []string + want map[string]any + }{ + { + name: "flags that name no metadata keep the instance's", + current: withMetadata, + args: inline, + wantCalls: []string{"GET", "PUT"}, + want: map[string]any{"region": "kanto", "seats": float64(40)}, + }, + { + name: "--metadata-json is sent as given, without reading the instance", + current: withMetadata, + args: append(append([]string{}, inline...), "--metadata-json", `{"region": "kansai"}`), + wantCalls: []string{"PUT"}, + want: map[string]any{"region": "kansai"}, + }, + { + name: "an empty --metadata-json clears the metadata", + current: withMetadata, + args: append(append([]string{}, inline...), "--metadata-json", `{}`), + wantCalls: []string{"PUT"}, + want: map[string]any{}, + }, + { + name: "an instance without metadata still sends an object", + current: withoutMetadata, + args: inline, + wantCalls: []string{"GET", "PUT"}, + want: map[string]any{}, + }, + { + name: "a payload is sent as written", + current: withMetadata, + args: []string{"--payload", `{ + "name": "Sakura Tokyo Prod", "description": "Production, Shibuya", "customerId": "cus_1", "licenseId": "lic_1", + "startLicenseDate": "2026-01-01T00:00:00Z", "endLicenseDate": "2027-01-01T00:00:00Z", + "metadata": {"region": "kansai"}}`}, + wantCalls: []string{"PUT"}, + want: map[string]any{"region": "kansai"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + const path = "/instances/sakura-tokyo-prod" + api := givenRoutes(t, map[string]route{ + "GET " + path: {status: http.StatusOK, body: test.current}, + "PUT " + path: {status: http.StatusNoContent}, + }) + givenIsolatedConfig(t) + + runSuccessfully(t, append([]string{"--base-url", api.URL, "instances", "update", "sakura-tokyo-prod"}, test.args...)...) + + calls := api.calls() + wantCalls := make([]string, len(test.wantCalls)) + for i, method := range test.wantCalls { + wantCalls[i] = method + " " + path + } + expectCalls(t, calls, wantCalls...) + expectBody(t, calls[len(calls)-1].body, map[string]any{ + "name": "Sakura Tokyo Prod", "description": "Production, Shibuya", "customerId": "cus_1", "licenseId": "lic_1", + "startLicenseDate": "2026-01-01T00:00:00Z", "endLicenseDate": "2027-01-01T00:00:00Z", + "metadata": test.want, + }) + }) + } +} + +// TestInstancesUpdateStopsWhenTheInstanceCannotBeRead keeps the read a precondition: an +// update that cannot learn the metadata to keep sends nothing rather than an empty object. +func TestInstancesUpdateStopsWhenTheInstanceCannotBeRead(t *testing.T) { + api := givenRoutes(t, map[string]route{ + "GET /instances/gone": {status: http.StatusNotFound, body: `{"status": 404, "title": "Not Found"}`}, + }) + + code, err := executeForExitCode(t, "--base-url", api.URL, "instances", "update", "gone", + "--name", "Gone", "--description", "Removed", "--customer-id", "cus_1", "--license-id", "lic_1", + "--start-license-date", "2026-01-01T00:00:00Z", "--end-license-date", "2027-01-01T00:00:00Z") + + if err == nil { + t.Fatal("Execute() error = nil, want the failed read reported") + } + expectExitCode(t, code, exitNotFound) + expectCalls(t, api.calls(), "GET /instances/gone") +} diff --git a/internal/cmd/licenses.go b/internal/cmd/licenses.go index c0003ef..3f8c576 100644 --- a/internal/cmd/licenses.go +++ b/internal/cmd/licenses.go @@ -81,7 +81,11 @@ func newLicensesCreateCommand() *cobra.Command { cmd := &cobra.Command{ Use: "create", Short: "Create a license from --file, --payload, or inline flags", - Args: cobra.NoArgs, + Long: "Create a license from --file, --payload, or inline flags. There is no --version: the\n" + + "API numbers the versions itself and refuses one it is sent. A license that opens a new\n" + + "family is version 1; one added to an existing family, named by familySlug or familyId\n" + + "in --file or --payload, takes the next number.", + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { inputValue, err := resolveInput(file, payload, inline.changed(cmd), inline.build(cmd)) if err != nil { @@ -112,7 +116,12 @@ func newLicensesUpdateCommand() *cobra.Command { cmd := &cobra.Command{ Use: "update ", Short: "Update a license from --file, --payload, or inline flags", - Args: cobra.ExactArgs(1), + Long: "Update a license from --file, --payload, or inline flags. The version of a license is\n" + + "assigned by the API when the license is created and never changes, so --version does\n" + + "not set it: it states the version the license is expected to have, and the API refuses\n" + + "the update (exit code 5) when the license has another one. Left out, the update is\n" + + "applied whatever the version.", + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { inputValue, err := resolveInput(file, payload, inline.changed(cmd), inline.build(cmd)) if err != nil { @@ -132,6 +141,7 @@ func newLicensesUpdateCommand() *cobra.Command { } addInputSourceFlags(cmd, &file, &payload, "license") inline.register(cmd) + inline.registerVersion(cmd) return cmd } @@ -346,11 +356,20 @@ func (f *licenseInputFlags) register(cmd *cobra.Command) { cmd.Flags().StringVar(&f.name, "name", "", "License name") cmd.Flags().StringVar(&f.description, "description", "", "License description") cmd.Flags().StringVar(&f.kind, "type", "", "License type") - cmd.Flags().StringVar(&f.version, "version", "", "License version") cmd.Flags().StringVar(&f.versionName, "version-name", "", "Human-readable license version name") cmd.Flags().BoolVar(&f.isDefault, "default", false, "Whether the license is the default") } +// registerVersion adds --version to update only, and as a check rather than a setting. The +// API numbers the versions of a license itself. Its create refuses any version it is sent, +// so the request the SDK builds there has no such field: on create the flag was required +// and then dropped. Its update accepts the version the license already has, refuses any +// other, and keeps the stored one when none is sent. +func (f *licenseInputFlags) registerVersion(cmd *cobra.Command) { + cmd.Flags().StringVar(&f.version, "version", "", + "Version the license is expected to have: the API checks it and never changes it") +} + // registerSlug adds --slug to create only. The API never renames a license: // its update accepts the slug already in the path and refuses any other, so the // flag could only restate the argument or be refused. @@ -368,7 +387,6 @@ func (f *licenseInputFlags) build(cmd *cobra.Command) func() (sdk.LicenseInput, {f.name, "name"}, {f.description, "description"}, {f.kind, "type"}, - {f.version, "version"}, } { if err := requiredInlineString(required.value, required.flag); err != nil { return sdk.LicenseInput{}, err diff --git a/internal/cmd/licenses_test.go b/internal/cmd/licenses_test.go index 54f12f1..1781795 100644 --- a/internal/cmd/licenses_test.go +++ b/internal/cmd/licenses_test.go @@ -2,6 +2,7 @@ package cmd import ( "net/http" + "strings" "testing" ) @@ -118,6 +119,90 @@ func TestLicensesEntitlementsAssociateSendsTheAllowance(t *testing.T) { } } +// TestLicensesCreateLeavesTheVersionToTheAPI pins create to what the API takes. It numbers +// the versions of a license itself and refuses any it is sent, so the request has no +// version field: --version used to be required there and then dropped. It is no longer a +// flag of create, so passing it is refused before anything is sent. +func TestLicensesCreateLeavesTheVersionToTheAPI(t *testing.T) { + api := givenRoutes(t, map[string]route{ + "POST /licenses": {status: http.StatusCreated, body: `{"name": "Premium", "slug": "premium", "version": "1"}`}, + }) + givenIsolatedConfig(t) + create := []string{ + "--base-url", api.URL, "licenses", "create", + "--name", "Premium", "--description", "Every feature.", "--type", "PAID", "--default=false", + } + + runSuccessfully(t, create...) + + expectCalls(t, api.calls(), "POST /licenses") + expectBody(t, api.calls()[0].body, map[string]any{ + "name": "Premium", "description": "Every feature.", "type": "PAID", "isDefault": false, + }) + + code, err := executeForExitCode(t, append(create, "--version", "2")...) + + if err == nil || !strings.Contains(err.Error(), "unknown flag: --version") { + t.Fatalf("Execute() error = %v, want --version refused as an unknown flag", err) + } + expectExitCode(t, code, exitUsage) + expectCalls(t, api.calls(), "POST /licenses") +} + +// TestLicensesUpdateSendsTheVersionAsACheck pins what is left of --version on update. The +// API accepts there the version the license already has and refuses any other, so the flag +// is sent as given for the API to check, and is no longer required: without it the version +// goes out empty, which the API reads as the one it holds. +func TestLicensesUpdateSendsTheVersionAsACheck(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + {name: "without --version the update names none", want: ""}, + {name: "with --version it is sent for the API to check", args: []string{"--version", "2"}, want: "2"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + api := givenRoutes(t, map[string]route{ + "PUT /licenses/premium-v2": {status: http.StatusNoContent}, + }) + givenIsolatedConfig(t) + + args := append([]string{ + "--base-url", api.URL, "licenses", "update", "premium-v2", + "--name", "Premium", "--description", "Every feature.", "--type", "PAID", "--default=false", + }, test.args...) + runSuccessfully(t, args...) + + expectCalls(t, api.calls(), "PUT /licenses/premium-v2") + expectBody(t, api.calls()[0].body, map[string]any{ + "name": "Premium", "description": "Every feature.", "type": "PAID", "isDefault": false, + "version": test.want, + }) + }) + } +} + +// TestLicensesUpdateReportsARefusedVersion checks the other end of the check: the API's +// refusal of a version the license does not have reaches the operator as a refused request. +func TestLicensesUpdateReportsARefusedVersion(t *testing.T) { + api := givenRoutes(t, map[string]route{ + "PUT /licenses/premium-v2": {status: http.StatusUnprocessableEntity, body: `{ + "status": 422, "title": "Unprocessable Entity", "code": "UpdateLicense.VersionNotSettable", + "detail": "License \"premium-v2\" is version 2; the version is assigned by the server and cannot be changed"}`}, + }) + + code, err := executeForExitCode(t, "--base-url", api.URL, "licenses", "update", "premium-v2", + "--name", "Premium", "--description", "Every feature.", "--type", "PAID", "--default=false", "--version", "3") + + if err == nil || !strings.Contains(err.Error(), "UpdateLicense.VersionNotSettable") { + t.Fatalf("Execute() error = %v, want the API's refusal", err) + } + expectExitCode(t, code, exitRejected) +} + func numberGrant(value float64) map[string]any { return map[string]any{"type": "number", "value": value} } diff --git a/internal/input/input.go b/internal/input/input.go index 15cc4f5..b4805f9 100644 --- a/internal/input/input.go +++ b/internal/input/input.go @@ -8,8 +8,6 @@ import ( "os" "path/filepath" "strings" - - "gopkg.in/yaml.v3" ) // LoadFile reads and decodes the JSON or YAML file at path into a value of type T. A path of "-" reads from stdin. @@ -57,27 +55,46 @@ func read(path string) ([]byte, error) { func decode[T any](data []byte, formatHint string) (T, error) { var zero T - var out T switch strings.ToLower(formatHint) { case ".json": + var out T if err := json.Unmarshal(data, &out); err != nil { return zero, fmt.Errorf("decode JSON input: %w", err) } return out, nil case ".yaml", ".yml": - if err := yaml.Unmarshal(data, &out); err != nil { + document, err := parseYAML(data) + if err != nil { + return zero, fmt.Errorf("decode YAML input: %w", err) + } + out, err := decodeYAML[T](document) + if err != nil { return zero, fmt.Errorf("decode YAML input: %w", err) } return out, nil } - if err := json.Unmarshal(data, &out); err == nil { - return out, nil - } - if err := yaml.Unmarshal(data, &out); err == nil { + var out T + jsonErr := json.Unmarshal(data, &out) + if jsonErr == nil { return out, nil } + // Not JSON, or JSON that does not fit the request. Either is tried as YAML, where a text + // field takes a bare number, and the second is reported as the JSON it is if that fails. + jsonValid := json.Valid(data) + if document, err := parseYAML(data); err == nil { + out, err := decodeYAML[T](document) + if err == nil { + return out, nil + } + if !jsonValid { + return zero, fmt.Errorf("decode YAML input: %w", err) + } + } + if jsonValid { + return zero, fmt.Errorf("decode JSON input: %w", jsonErr) + } return zero, fmt.Errorf("decode input: expected JSON or YAML") } diff --git a/internal/input/input_test.go b/internal/input/input_test.go index c93f286..a25ec9d 100644 --- a/internal/input/input_test.go +++ b/internal/input/input_test.go @@ -1,10 +1,13 @@ package input import ( + "encoding/json" "os" "path/filepath" + "reflect" "strings" "testing" + "time" ) type component struct { @@ -171,6 +174,237 @@ func TestLoadString(t *testing.T) { } } +// instanceInput stands for an input type of the API client. Its fields carry no tags: a key +// reaches a field the way JSON matches one, by name and whatever the case. +type instanceInput struct { + Name string + Version string + CustomerID string + DeploymentZoneID *string + IsDefault bool + SaleUnitFactor *float64 + StartLicenseDate time.Time + GroupSlugs []string + Metadata map[string]any +} + +// flagInput stands for a type the API client generates: its keys are spelled by json tags. +type flagInput struct { + EventName string `json:"event_name"` + DefaultVariant typedValue `json:"default_variant"` +} + +// typedValue stands for a value discriminated by its type field. It reads itself from JSON +// and from nothing else. +type typedValue struct { + union json.RawMessage +} + +func (v *typedValue) UnmarshalJSON(data []byte) error { + v.union = append(v.union[:0], data...) + return nil +} + +// TestYAMLDecodesToWhatTheEquivalentJSONDecodesTo pins a YAML document to the request the +// same document written in JSON makes. The types of the API name their fields for JSON +// only, so a YAML decoder reading them matched the keys of one word and dropped every other +// without a word -- externalCustomerId, default_variant -- and read a typed value as nothing. +func TestYAMLDecodesToWhatTheEquivalentJSONDecodesTo(t *testing.T) { + t.Parallel() + + const instanceYAML = ` +name: Sakura Tokyo Prod +customerId: cus_1 +deploymentZoneId: zone_1 +isDefault: true +saleUnitFactor: 10 +startLicenseDate: 2026-01-01T00:00:00Z +groupSlugs: [restaurant-operations, delivery] +metadata: + region: kanto + seats: 40 + ratio: 0.5 + pilot: false + owner: null + opened: 2026-01-01 + tags: [a, b] +` + const instanceJSON = `{ + "name": "Sakura Tokyo Prod", "customerId": "cus_1", "deploymentZoneId": "zone_1", "isDefault": true, + "saleUnitFactor": 10, "startLicenseDate": "2026-01-01T00:00:00Z", + "groupSlugs": ["restaurant-operations", "delivery"], + "metadata": {"region": "kanto", "seats": 40, "ratio": 0.5, "pilot": false, "owner": null, + "opened": "2026-01-01", "tags": ["a", "b"]} +}` + expectSameDecoding[instanceInput](t, "an input type, its keys of several words included", instanceYAML, instanceJSON) + + expectSameDecoding[flagInput](t, "a generated type and the typed value it holds", + "event_name: delivery-tracking\ndefault_variant:\n type: boolean\n value: false\n", + `{"event_name": "delivery-tracking", "default_variant": {"type": "boolean", "value": false}}`) + + expectSameDecoding[typedValue](t, "a typed value on its own", + "type: number\nvalue: 2000\n", `{"type": "number", "value": 2000}`) + + expectSameDecoding[map[string]any](t, "a free-form object", + "max: 10\n1024: kept\nnested:\n enabled: true\n", `{"max": 10, "1024": "kept", "nested": {"enabled": true}}`) +} + +// TestYAMLKeepsTextAsWritten covers the two places where YAML and JSON part ways on what a +// value is. A text field takes what was written: version 1.0 is the text 1.0, not the +// number 1, which JSON would refuse there. And a date stays the text it is in JSON instead +// of becoming a time and coming back respelled. +func TestYAMLKeepsTextAsWritten(t *testing.T) { + t.Parallel() + + got, err := LoadString[instanceInput]("name: 2026\nversion: 1.0\ncustomerId: 2026-08-01\nmetadata:\n since: 2026-08-01\n") + if err != nil { + t.Fatalf("LoadString() error = %v", err) + } + + want := instanceInput{ + Name: "2026", + Version: "1.0", + CustomerID: "2026-08-01", + Metadata: map[string]any{"since": "2026-08-01"}, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("LoadString() = %+v, want %+v", got, want) + } +} + +// TestYAMLRejectsWhatItCannotPlace keeps a YAML document from going out incomplete: a key +// the request has no field for is an error that names it, and so is a value of the wrong +// kind. Neither is dropped. +func TestYAMLRejectsWhatItCannotPlace(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + content string + wantErr string + }{ + { + name: "a key the request has no field for", + content: "name: api\nexternal_customer_id: crm-1024\n", + wantErr: `decode YAML input: unknown field "external_customer_id"`, + }, + { + name: "a value of the wrong kind", + content: "name: api\nisDefault: sometimes\n", + wantErr: "decode YAML input: cannot unmarshal string", + }, + { + name: "a key that is a list", + content: "name: api\nmetadata:\n [a, b]: c\n", + wantErr: "decode YAML input: yaml: invalid map key", + }, + { + name: "a key tagged as a number", + content: "name: api\nmetadata:\n !!int 5: c\n", + wantErr: "decode YAML input: a key must be text", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + // By extension and by content alike: the reason is not lost to the sniffing. + _, fileErr := LoadFile[instanceInput](givenFile(t, "instance.yaml", test.content)) + _, stringErr := LoadString[instanceInput](test.content) + + for source, err := range map[string]error{"LoadFile": fileErr, "LoadString": stringErr} { + if err == nil || !strings.Contains(err.Error(), test.wantErr) { + t.Errorf("%s() error = %v, want it to contain %q", source, err, test.wantErr) + } + } + }) + } +} + +// TestJSONIgnoresAKeyTheRequestHasNoFieldFor records that JSON input is left as it was: the +// keys a request has no field for are skipped, which is what lets a resource printed by +// --output json be edited and written back with the fields only the API sets still in it. +func TestJSONIgnoresAKeyTheRequestHasNoFieldFor(t *testing.T) { + t.Parallel() + + got, err := LoadString[instanceInput](`{"id": "ins_1", "name": "Sakura Tokyo Prod", "createdAt": "2026-10-01T09:00:00Z"}`) + if err != nil { + t.Fatalf("LoadString() error = %v", err) + } + if want := (instanceInput{Name: "Sakura Tokyo Prod"}); !reflect.DeepEqual(got, want) { + t.Fatalf("LoadString() = %+v, want %+v", got, want) + } +} + +// TestAnEmptyYAMLDocumentDecodesToNothing keeps an empty file what it was: no field set, +// and no error of its own -- the command reports what the request is missing. +func TestAnEmptyYAMLDocumentDecodesToNothing(t *testing.T) { + t.Parallel() + + got, err := LoadFile[instanceInput](givenFile(t, "instance.yaml", "# nothing yet\n")) + if err != nil { + t.Fatalf("LoadFile() error = %v", err) + } + if !reflect.DeepEqual(got, instanceInput{}) { + t.Fatalf("LoadFile() = %+v, want the zero value", got) + } +} + +// expectSameDecoding decodes one document written both ways and fails unless the two +// requests are the same, and the YAML one is not empty. +func expectSameDecoding[T any](t *testing.T, name, yamlDocument, jsonDocument string) { + t.Helper() + + fromJSON, err := LoadFile[T](givenFile(t, "document.json", jsonDocument)) + if err != nil { + t.Fatalf("%s: LoadFile(json) error = %v", name, err) + } + fromYAML, err := LoadFile[T](givenFile(t, "document.yaml", yamlDocument)) + if err != nil { + t.Fatalf("%s: LoadFile(yaml) error = %v", name, err) + } + sniffed, err := LoadString[T](yamlDocument) + if err != nil { + t.Fatalf("%s: LoadString(yaml) error = %v", name, err) + } + + var zero T + if reflect.DeepEqual(fromJSON, zero) { + t.Fatalf("%s: the json document decoded to nothing", name) + } + want := normalized(t, fromJSON) + if !reflect.DeepEqual(normalized(t, fromYAML), want) || !reflect.DeepEqual(normalized(t, sniffed), want) { + t.Fatalf("%s: yaml decoded to %+v (by extension) and %+v (by content), want what json decodes to, %+v", + name, fromYAML, sniffed, fromJSON) + } +} + +// normalized returns value with the raw JSON a typedValue holds decoded, so that two +// spellings of one document compare equal. +func normalized(t *testing.T, value any) any { + t.Helper() + + switch typed := value.(type) { + case typedValue: + return decodedJSON(t, typed.union) + case flagInput: + return [2]any{typed.EventName, decodedJSON(t, typed.DefaultVariant.union)} + default: + return value + } +} + +func decodedJSON(t *testing.T, raw json.RawMessage) any { + t.Helper() + + var decoded any + if err := json.Unmarshal(raw, &decoded); err != nil { + t.Fatalf("json.Unmarshal(%q) error = %v", raw, err) + } + return decoded +} + func givenFile(t *testing.T, name, content string) string { t.Helper() diff --git a/internal/input/yaml.go b/internal/input/yaml.go new file mode 100644 index 0000000..7f8d6e0 --- /dev/null +++ b/internal/input/yaml.go @@ -0,0 +1,171 @@ +package input + +import ( + "bytes" + "encoding/json" + "errors" + "reflect" + "strings" + + "gopkg.in/yaml.v3" +) + +const ( + yamlMergeTag = "!!merge" + yamlNullTag = "!!null" + yamlStringTag = "!!str" + yamlTimestampTag = "!!timestamp" +) + +// parseYAML reads data as one YAML document. It fails on what is not YAML at all, and on +// nothing else: whether the document fits the request is decodeYAML's to say. +func parseYAML(data []byte) (*yaml.Node, error) { + var document yaml.Node + if err := yaml.Unmarshal(data, &document); err != nil { + return nil, err + } + return &document, nil +} + +// decodeYAML decodes document into a value of type T as the JSON document it stands for. +// +// The types of the API describe themselves in JSON only: the names of their fields come +// from json tags or from matching a key whatever its case, and a typed value reads itself +// through UnmarshalJSON. A YAML decoder reads none of that. It matched the keys of one word +// by accident and dropped every other without an error -- externalCustomerId, default_variant +// -- so the request went out incomplete. Decoding the JSON the document stands for gives a +// YAML file the request the same file written in JSON makes. +// +// A key the request has no field for is an error rather than a drop, which is the failure +// this replaces. +func decodeYAML[T any](document *yaml.Node) (T, error) { + var zero T + + // An empty document has no node to decode, and stands for null. + var generic any + if document.Kind != 0 { + keepText(document, reflect.TypeFor[T]()) + if err := document.Decode(&generic); err != nil { + return zero, err + } + } + + raw, err := json.Marshal(generic) + if err != nil { + var unsupported *json.UnsupportedTypeError + if errors.As(err, &unsupported) { + // The one thing a YAML document holds that JSON cannot: a mapping whose keys are + // not text. + return zero, errors.New("a key must be text") + } + return zero, asYAMLError(err) + } + + var out T + decoder := json.NewDecoder(bytes.NewReader(raw)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&out); err != nil { + return zero, asYAMLError(err) + } + return out, nil +} + +// asYAMLError reports a failure to read the JSON a YAML document stands for without naming +// JSON, which the operator did not write. +func asYAMLError(err error) error { + return errors.New(strings.TrimPrefix(err.Error(), "json: ")) +} + +// keepText marks as text the plain scalars of node that reading the document as JSON would +// otherwise change, so that each reaches the request as it was written: +// +// - a key, since the keys of a JSON object are text: 1024 is the key "1024"; +// - a date or a time, which JSON has no type for and carries as text: left alone it +// becomes a time and comes back respelled, 2026-08-01 as 2026-08-01T00:00:00Z; +// - a value bound for a text field: version: 1.0 is the text 1.0 there, not the number 1 +// that JSON refuses for it. +// +// bound is the Go type the node is decoded into, or nil when nothing is known of it. A +// quoted or explicitly tagged scalar already says what it is and is left alone. +func keepText(node *yaml.Node, bound reflect.Type) { + for bound != nil && bound.Kind() == reflect.Pointer { + bound = bound.Elem() + } + if bound != nil && reflect.PointerTo(bound).Implements(reflect.TypeFor[json.Unmarshaler]()) { + // The type reads its own JSON: what it holds is not for its Go fields to tell. + bound = nil + } + + switch node.Kind { + case yaml.DocumentNode: + for _, content := range node.Content { + keepText(content, bound) + } + case yaml.SequenceNode: + for _, item := range node.Content { + keepText(item, elementOf(bound)) + } + case yaml.MappingNode: + for i := 0; i+1 < len(node.Content); i += 2 { + key, value := node.Content[i], node.Content[i+1] + if key.Kind == yaml.ScalarNode && key.Tag != yamlMergeTag && isPlain(key) { + key.Tag = yamlStringTag + } + keepText(value, memberOf(bound, key.Value)) + } + case yaml.ScalarNode: + if !isPlain(node) || node.Tag == yamlNullTag { + return + } + if node.Tag == yamlTimestampTag || (bound != nil && bound.Kind() == reflect.String) { + node.Tag = yamlStringTag + } + } +} + +// isPlain reports whether a scalar was written bare: neither quoted nor given a tag, which +// are the two ways a document says what a value is. +func isPlain(node *yaml.Node) bool { + const explicit = yaml.TaggedStyle | yaml.DoubleQuotedStyle | yaml.SingleQuotedStyle | yaml.LiteralStyle | yaml.FoldedStyle + return node.Style&explicit == 0 +} + +// elementOf returns the type of the items of a list type, or nil. +func elementOf(bound reflect.Type) reflect.Type { + if bound == nil { + return nil + } + if bound.Kind() == reflect.Slice || bound.Kind() == reflect.Array { + return bound.Elem() + } + return nil +} + +// memberOf returns the type of what key holds in a map or a struct type, or nil. For a +// struct it is the field JSON gives the key to: the one named by its json tag, or by its +// own name, whatever the case. +func memberOf(bound reflect.Type, key string) reflect.Type { + if bound == nil { + return nil + } + + switch bound.Kind() { + case reflect.Map: + return bound.Elem() + case reflect.Struct: + for i := range bound.NumField() { + field := bound.Field(i) + if !field.IsExported() { + continue + } + name, _, _ := strings.Cut(field.Tag.Get("json"), ",") + if name == "" { + name = field.Name + } + if strings.EqualFold(name, key) { + return field.Type + } + } + } + return nil +} diff --git a/internal/output/output.go b/internal/output/output.go index 9f54bce..ad0cac3 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -82,7 +82,11 @@ func writeJSON(w io.Writer, value any) error { } func writeYAML(w io.Writer, value any) error { - data, err := yaml.Marshal(value) + document, err := jsonDocument(value) + if err != nil { + return err + } + data, err := yaml.Marshal(document) if err != nil { return err } diff --git a/internal/output/yaml.go b/internal/output/yaml.go new file mode 100644 index 0000000..2a59e1d --- /dev/null +++ b/internal/output/yaml.go @@ -0,0 +1,69 @@ +package output + +import ( + "bytes" + "encoding/json" + + "gopkg.in/yaml.v3" +) + +// jsonDocument returns value as the YAML tree of its JSON encoding. The API's types describe +// themselves in JSON only: the names of their fields, the fields they leave out and the +// typed values they hold are spelled by json tags and MarshalJSON methods, none of which a +// YAML encoder reads. Left to itself it names a field after the Go one (externalcustomerid), +// prints every absent field as null and a typed value as an empty mapping. Going through +// JSON makes --output yaml the document --output json prints, key for key and in the same +// order. +func jsonDocument(value any) (*yaml.Node, error) { + raw, err := json.Marshal(value) + if err != nil { + return nil, err + } + + decoder := json.NewDecoder(bytes.NewReader(raw)) + // A number is carried over as written, never through a float64 that would round a + // large integer or respell a decimal. + decoder.UseNumber() + + return jsonNode(decoder) +} + +// jsonNode reads the next JSON value from decoder as a YAML node. +func jsonNode(decoder *json.Decoder) (*yaml.Node, error) { + token, err := decoder.Token() + if err != nil { + return nil, err + } + + switch typed := token.(type) { + case json.Delim: + node := &yaml.Node{Kind: yaml.MappingNode} + if typed == '[' { + node.Kind = yaml.SequenceNode + } + // An object's tokens alternate between a key and its value, which is the order a + // mapping node holds them in, so one loop reads an object and an array alike. + for decoder.More() { + child, err := jsonNode(decoder) + if err != nil { + return nil, err + } + node.Content = append(node.Content, child) + } + // The closing delimiter. + if _, err := decoder.Token(); err != nil { + return nil, err + } + return node, nil + case json.Number: + return &yaml.Node{Kind: yaml.ScalarNode, Value: typed.String()}, nil + default: + // A string, a boolean or null: the encoder knows how each is written, and which + // strings need quoting to stay strings. + node := &yaml.Node{} + if err := node.Encode(typed); err != nil { + return nil, err + } + return node, nil + } +} diff --git a/internal/output/yaml_test.go b/internal/output/yaml_test.go new file mode 100644 index 0000000..12317aa --- /dev/null +++ b/internal/output/yaml_test.go @@ -0,0 +1,150 @@ +package output + +import ( + "bytes" + "encoding/json" + "math" + "reflect" + "testing" + "time" + + "gopkg.in/yaml.v3" +) + +// apiResource stands for a type of the API client. It describes itself in JSON only: json +// tags, an optional field that is left out when absent, and a typed value that knows how to +// write itself as JSON and nothing else. +type apiResource struct { + ID string `json:"id"` + ExternalCustomerID *string `json:"externalCustomerId,omitempty"` + Domain *string `json:"domain,omitempty"` + CreatedAt time.Time `json:"createdAt"` + Value typedValue `json:"value"` +} + +type typedValue struct { + union json.RawMessage +} + +func (v typedValue) MarshalJSON() ([]byte, error) { + return v.union, nil +} + +// TestWriteYAMLPrintsTheJSONDocument pins what YAML output is made of: the keys, the values +// and the key order of the JSON encoding. A YAML encoder reading the Go type instead names +// the fields externalcustomerid and createdat, prints the absent domain as null and the +// typed value as an empty mapping. +func TestWriteYAMLPrintsTheJSONDocument(t *testing.T) { + t.Parallel() + + externalID := "crm-1024" + resource := apiResource{ + ID: "cus_1", + ExternalCustomerID: &externalID, + CreatedAt: time.Date(2026, time.October, 1, 9, 0, 0, 0, time.UTC), + Value: typedValue{union: json.RawMessage(`{"type":"number","value":1000}`)}, + } + + var out bytes.Buffer + if err := Write(&out, FormatYAML, resource, Table{}); err != nil { + t.Fatalf("Write() error = %v", err) + } + + const want = `id: cus_1 +externalCustomerId: crm-1024 +createdAt: "2026-10-01T09:00:00Z" +value: + type: number + value: 1000 +` + if out.String() != want { + t.Fatalf("Write() output =\n%s\nwant\n%s", out.String(), want) + } +} + +// TestWriteYAMLKeepsEveryValueWhatItIsInJSON checks the values one kind at a time: read +// back, the YAML document is the JSON one. A string that reads as another type stays a +// string, and a number is printed as JSON writes it. +func TestWriteYAMLKeepsEveryValueWhatItIsInJSON(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + value any + want string + }{ + {name: "a plain string", value: "acme", want: "acme\n"}, + {name: "a string that reads as a boolean", value: "true", want: "\"true\"\n"}, + {name: "a string that reads as a number", value: "1024", want: "\"1024\"\n"}, + {name: "a string that reads as null", value: "null", want: "\"null\"\n"}, + {name: "an empty string", value: "", want: "\"\"\n"}, + {name: "a string of several lines", value: "first\nsecond", want: "|-\n first\n second\n"}, + {name: "a boolean", value: false, want: "false\n"}, + {name: "null", value: nil, want: "null\n"}, + {name: "an integer", value: 1000, want: "1000\n"}, + {name: "a decimal", value: 730.5, want: "730.5\n"}, + {name: "an integer too large for a float", value: uint64(math.MaxUint64), want: "18446744073709551615\n"}, + {name: "an empty object", value: map[string]any{}, want: "{}\n"}, + {name: "an empty list", value: []string{}, want: "[]\n"}, + {name: "an absent list", value: []string(nil), want: "null\n"}, + { + name: "a list of objects", + value: []map[string]any{{"slug": "monthly-orders", "limit": -1}}, + want: "- limit: -1\n slug: monthly-orders\n", + }, + { + name: "an object holding a list", + value: map[string]any{"scopes": []string{"read:instances", "write:instances"}}, + want: "scopes:\n - read:instances\n - write:instances\n", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + var out bytes.Buffer + if err := Write(&out, FormatYAML, test.value, Table{}); err != nil { + t.Fatalf("Write() error = %v", err) + } + if out.String() != test.want { + t.Fatalf("Write() output = %q, want %q", out.String(), test.want) + } + + var fromYAML any + if err := yaml.Unmarshal(out.Bytes(), &fromYAML); err != nil { + t.Fatalf("yaml.Unmarshal() error = %v; output = %q", err, out.String()) + } + if got, want := asJSON(t, fromYAML), asJSON(t, test.value); !reflect.DeepEqual(got, want) { + t.Fatalf("read back, the yaml document is %v, want %v", got, want) + } + }) + } +} + +// TestWriteYAMLReportsAValueJSONCannotEncode keeps a value with no JSON form an error +// rather than an empty document. +func TestWriteYAMLReportsAValueJSONCannotEncode(t *testing.T) { + t.Parallel() + + var out bytes.Buffer + if err := Write(&out, FormatYAML, map[string]any{"ratio": math.NaN()}, Table{}); err == nil { + t.Fatalf("Write() error = nil, want one; output = %q", out.String()) + } +} + +// asJSON returns value as JSON types it, so that a document read from YAML compares with +// the value it was written from. +func asJSON(t *testing.T, value any) any { + t.Helper() + + raw, err := json.Marshal(value) + if err != nil { + t.Fatalf("json.Marshal() error = %v", err) + } + var out any + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatalf("json.Unmarshal() error = %v", err) + } + return out +}