From 452f0d33800c0a53bae19cc4cf73c8370bc6b4e0 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:13:30 +0100 Subject: [PATCH] fix: a created resource answers with what names it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every generated resource failed at apply, and each fix uncovered the next. The request body carried invented values. Fixture derivation read enum, advisory values and format, and never the document's own example — the only thing a document says about a string whose shape it otherwise leaves to prose. A URL property declaring no format became "tfpfgen-test-endpoint-url" and the API refused it. The example now wins over an invented name, and a number moves inside whatever bounds the document declares rather than always being 7. One synthesised name is restored when preferring examples would otherwise leave an entity with no prefixed string, because cleanup matches a live object by that prefix and one is enough. That reached a wire fixture whose example carries a backtick, which a raw Go literal has no escape for, so the generated file stopped being Go at that character. The literal is now rendered in Go — raw where the value permits, interpreted where it does not — and the template embeds one finished expression. With bodies the API accepts, creates began succeeding and failing later: the framework refuses a resource that declares an identity schema and returns without setting the identity. Thirty-four declared one and none wrote it. Create and Read now write it beside the state. A create whose response is a type of its own was discarded whole, so the settling read addressed the object by an empty string. The id is taken from that response where the same accessor answers the same type on both, which is what makes the read's conversion right for it. Four changes rather than four commits: they share render_resource.go and the crud template, and splitting them there would mean staging half a file. Co-Authored-By: Claude Opus 5 (1M context) --- internal/emit/render_datasource.go | 2 +- internal/emit/render_identity.go | 16 +++ internal/emit/render_resource.go | 56 +++++++- internal/emit/render_resource_test.go | 30 +++++ internal/fixtures/fixtures.go | 121 ++++++++++++++++-- internal/fixtures/fixtures_test.go | 96 ++++++++++++++ .../intermediate_representation/attributes.go | 10 +- .../constraints_test.go | 68 ++++++++++ internal/intermediate_representation/model.go | 6 + internal/sdkbind/model.go | 7 + internal/sdkbind/prune.go | 42 ++++++ .../services/datasource/responders.go.tmpl | 2 +- .../templates/services/resource/crud.go.tmpl | 13 ++ .../services/resource/responders.go.tmpl | 2 +- 14 files changed, 452 insertions(+), 19 deletions(-) create mode 100644 internal/emit/render_resource_test.go diff --git a/internal/emit/render_datasource.go b/internal/emit/render_datasource.go index cbd840d..c6771c3 100644 --- a/internal/emit/render_datasource.go +++ b/internal/emit/render_datasource.go @@ -459,7 +459,7 @@ func (e *serviceRenderer) datasourceModelImports(models string) string { // datasourceMocks fills the responder context. func (e *serviceRenderer) datasourceMocks(d *datasourceData, ds *ir.Datasource, spec fixtures.Fixture) { d.RegistryName = ds.Names.TerraformType + ".data" - d.ResponseMaximal = string(spec.WireJSON(fixtures.ResponseMaximal)) + d.ResponseMaximal = goStringLiteral(string(spec.WireJSON(fixtures.ResponseMaximal))) d.ListPayload = listPayloadExpr(ds.ListEnvelopeKey, "[]map[string]any{object()}") if ds.Operations.List != nil { // A parent-scoped collection is requested with its addressing diff --git a/internal/emit/render_identity.go b/internal/emit/render_identity.go index b600077..804bf4f 100644 --- a/internal/emit/render_identity.go +++ b/internal/emit/render_identity.go @@ -142,3 +142,19 @@ func identityModelFields(identity []identityAttribute) string { func identityValueType(kind ir.AttributeType) string { return scalarSchemaType(kind).ValueType } + +// identitySetLines renders the statements that write a resource's identity +// from its model, indented to depth. +// +// The framework requires a resource declaring an identity schema to answer +// with the identity as well as the state: a create or read that leaves it +// unset is refused as a provider fault, whatever the API did. +func identitySetLines(identity []identityAttribute, model string, depth int) string { + indent := strings.Repeat("\t", depth) + var b strings.Builder + for _, attribute := range identity { + fmt.Fprintf(&b, "%sresp.Diagnostics.Append(resp.Identity.SetAttribute(ctx, path.Root(%q), %s.%s)...)\n", + indent, attribute.Name, model, ir.GoName(attribute.Name)) + } + return b.String() +} diff --git a/internal/emit/render_resource.go b/internal/emit/render_resource.go index 1eabd23..0d2ec8b 100644 --- a/internal/emit/render_resource.go +++ b/internal/emit/render_resource.go @@ -45,6 +45,10 @@ type resourceData struct { // IdentityAttributes is the identity schema's attribute declarations, // empty for a resource nothing lists. IdentityAttributes string + // IdentitySets writes the identity beside the state. The framework + // refuses a create or read that declares an identity schema and leaves + // the identity unset, so the two are emitted together or not at all. + IdentitySets string SchemaDescription string SchemaAttributes string @@ -75,9 +79,12 @@ type resourceData struct { UpdatePlan callPlan DeletePlan callPlan CreateMapsResponse bool - UpdateMapsResponse bool - UpdateParamCopies string - MissingUpdate bool + // CreateIDFromResponse assigns the new object's id from a create + // response that is not the read model, and so is not mapped wholesale. + CreateIDFromResponse string + UpdateMapsResponse bool + UpdateParamCopies string + MissingUpdate bool HasEC bool ECDuration string @@ -248,6 +255,7 @@ func (e *serviceRenderer) resourceCode(d *resourceData, r *ir.Resource, rb *sdkb if identity := resourceIdentity(r); len(identity) > 0 { e.identities[r.Names.Key] = identity d.IdentityAttributes = identitySchemaDecls(identity, 3) + d.IdentitySets = identitySetLines(identity, "data", 1) imports.add("identityschema", "github.com/hashicorp/terraform-plugin-framework/resource/identityschema") } } @@ -378,6 +386,24 @@ func (e *serviceRenderer) resourceCRUD(d *resourceData, rb *sdkbind.ResourceBind if d.CreateMapsResponse { createPayload = "created" } + // The create answers a type of its own, so nothing maps it — but it + // still carries the id, and the settling read has no other way to + // address what was just made. + if !d.CreateMapsResponse && rb.CreateIDAccess != "" { + for _, n := range nodes { + if n.attr.Name != idAttributeName || n.fb == nil { + continue + } + fn, cerr := readConvert(n.fb) + if cerr != nil { + break + } + createPayload = "created" + d.CreateIDFromResponse = fmt.Sprintf("data.%s = convert.%s(created.%s())", + ir.GoName(idAttributeName), fn, rb.CreateIDAccess) + break + } + } if d.CreatePlan, err = buildCallPlan(createCall, createPayload, nodes, "data", respDiagnostics()); err != nil { return fmt.Errorf("create: %w", err) } @@ -424,6 +450,12 @@ func (e *serviceRenderer) resourceCRUD(d *resourceData, rb *sdkbind.ResourceBind if d.Singleton { imports.add("", "github.com/hashicorp/terraform-plugin-framework/types") } + if d.IdentitySets != "" { + imports.add("", "github.com/hashicorp/terraform-plugin-framework/path") + } + if d.CreateIDFromResponse != "" { + imports.add("", e.pc.Module+"/internal/services/common/convert") + } e.addSDKImports(imports, d.CreatePlan.Assign, d.ReadPlan.Assign, d.DeletePlan.ClosureBody, d.UpdatePlan.Assign) addPlanImports(imports, d.CreatePlan, d.ReadPlan, d.UpdatePlan, d.DeletePlan) d.CRUDImports = imports.render() @@ -493,8 +525,8 @@ func (e *serviceRenderer) resourceMocks(d *resourceData, r *ir.Resource, rb *sdk return unrenderable("the read path %s declares no parameter segment for the mock to key on", r.Operations.Read.PathTemplate) } d.IDWire = idWire(rb.Fields) - d.ResponseMinimal = string(spec.WireJSON(fixtures.ResponseMinimal)) - d.ResponseMaximal = string(spec.WireJSON(fixtures.ResponseMaximal)) + d.ResponseMinimal = goStringLiteral(string(spec.WireJSON(fixtures.ResponseMinimal))) + d.ResponseMaximal = goStringLiteral(string(spec.WireJSON(fixtures.ResponseMaximal))) d.CreateStatus = successStatus(r.Operations.Create, 201) d.DeleteStatus = successStatus(r.Operations.Delete, 204) d.HasDelete = true @@ -535,6 +567,20 @@ func successStatus(op *ir.Operation, fallback int) int { const unitEndpoint = "https://unit.invalid" // mockURL is the literal URL one collection operation answers on. +// goStringLiteral renders a value as a finished Go string literal, its +// delimiters included, so a template embeds one expression rather than +// wrapping a value in quotes it cannot reason about. +// +// A raw literal keeps multi-line wire JSON legible, but no escape exists +// inside one, so a value carrying a backtick — a document's own example may — +// is spelled as an interpreted literal instead of ending the literal early. +func goStringLiteral(value string) string { + if !strings.Contains(value, "`") { + return "`" + value + "`" + } + return strconv.Quote(value) +} + func mockURL(pathTemplate string) string { return unitEndpoint + pathTemplate } diff --git a/internal/emit/render_resource_test.go b/internal/emit/render_resource_test.go new file mode 100644 index 0000000..48d448a --- /dev/null +++ b/internal/emit/render_resource_test.go @@ -0,0 +1,30 @@ +package emit + +import ( + "strconv" + "strings" + "testing" +) + +// TestUnit_Emit_AWireFixtureCarryingABacktickStaysOneLiteral pins the literal +// a value cannot end early. A document's own example may contain a backtick, +// and a raw literal has no escape for one, so the generated file would stop +// being Go at that character. +func TestUnit_Emit_AWireFixtureCarryingABacktickStaysOneLiteral(t *testing.T) { + plain := goStringLiteral("{\n \"id\": \"x\"\n}") + if plain != "`{\n \"id\": \"x\"\n}`" { + t.Errorf("a value with no backtick = %s, want a raw literal", plain) + } + + quoted := goStringLiteral("the pattern `/` never matches") + if strings.HasPrefix(quoted, "`") { + t.Fatalf("a value carrying a backtick = %s, want an interpreted literal", quoted) + } + unquoted, err := strconv.Unquote(quoted) + if err != nil { + t.Fatalf("the rendered literal does not parse as one: %v", err) + } + if unquoted != "the pattern `/` never matches" { + t.Errorf("round trip = %q", unquoted) + } +} diff --git a/internal/fixtures/fixtures.go b/internal/fixtures/fixtures.go index de9fb9f..2f0ef76 100644 --- a/internal/fixtures/fixtures.go +++ b/internal/fixtures/fixtures.go @@ -73,6 +73,12 @@ type Entry struct { // Nested are the field values of an object attribute, or of the one // element a list of objects carries. Nested []Entry + + // synthesised is the prefixed name this entry would carry had the + // document declared no example, and is empty unless the example is what + // displaced it. The prefix guard restores it when nothing else in the + // entity carries the prefix. + synthesised string } // Omission is one attribute that has no fixture value, and why. @@ -111,9 +117,55 @@ func Derive(tree *ir.AttributeTree) Fixture { } s.Entries, s.Omissions = deriveTree(tree, nil) s.applyVariant(tree) + s.keepOnePrefixed() return s } +// keepOnePrefixed restores one synthesised name when preferring declared +// examples has left the entity with no prefixed string at all. +// +// The audit's cleanup contract matches a live object by any one of its string +// fields carrying the prefix, so one is enough — and an entity whose every +// string is an example is otherwise indistinguishable from an object the +// toolkit did not create, which the prefix pass must never delete. +func (s *Fixture) keepOnePrefixed() { + if len(s.Entries) == 0 || anyPrefixed(s.Entries) { + return + } + // The first displaced name, in attribute-tree order, so the choice is a + // function of the document rather than of which field happens to be + // nameable. + restoreFirstSynthesised(s.Entries) +} + +// anyPrefixed reports whether any scalar in the tree carries the prefix. +func anyPrefixed(values []Entry) bool { + for _, v := range values { + if s, ok := v.Scalar.(string); ok && strings.HasPrefix(s, NamePrefix) { + return true + } + if anyPrefixed(v.Nested) { + return true + } + } + return false +} + +// restoreFirstSynthesised puts back the first entry's invented name and +// reports whether it found one to put back. +func restoreFirstSynthesised(values []Entry) bool { + for i := range values { + if values[i].synthesised != "" { + values[i].Scalar = values[i].synthesised + return true + } + if restoreFirstSynthesised(values[i].Nested) { + return true + } + } + return false +} + // applyVariant reads the tree's conditional-edge facts and, when the entity is // multi-variant, pins the discriminator to one value and records which // top-level attributes that value excludes and which it forces. A @@ -328,9 +380,9 @@ func deriveTree(tree *ir.AttributeTree, path []string) ([]Entry, []Omission) { v.Nested = nested skips = append(skips, nestedSkips...) case a.Kind == ir.TypeList: - v.Scalar = scalarFor(a.ElementType, a, at) + v.Scalar, v.synthesised = scalarFor(a.ElementType, a, at) default: - v.Scalar = scalarFor(a.Kind, a, at) + v.Scalar, v.synthesised = scalarFor(a.Kind, a, at) } values = append(values, v) } @@ -364,27 +416,76 @@ func unifyByWire(values []Entry) { // declares values, format-driven when it declares what the string holds, // type-driven otherwise. A plain string carries the test prefix and the // attribute path so no two attributes share a value. -func scalarFor(kind ir.AttributeType, a ir.Attribute, path []string) any { +func scalarFor(kind ir.AttributeType, a ir.Attribute, path []string) (any, string) { switch kind { case ir.TypeBool: - return true + if b, ok := a.Example.(bool); ok { + return b, "" + } + return true, "" case ir.TypeInt64: - return int64(7) + return int64(boundedNumber(a, 7)), "" case ir.TypeFloat64: - return 1.5 + return boundedNumber(a, 1.5), "" default: if len(a.OneOf) > 0 { - return a.OneOf[0] + return a.OneOf[0], "" } if len(a.AdvisoryValues) > 0 { - return a.AdvisoryValues[0] + return a.AdvisoryValues[0], "" } name := NamePrefix + strings.ReplaceAll(strings.Join(path, "-"), "_", "-") if formatted, ok := formatValue(a.Format, name); ok { - return formatted + return formatted, "" + } + // The document declared no format, so the invented name is the only + // thing saying what the value looks like — and it says "a string", + // which an API that wanted a URL refuses. An example is the vendor's + // own statement of a value that is accepted, so it wins; the name it + // displaces is kept for the prefix guard. + if example, ok := a.Example.(string); ok && example != "" { + return example, name } - return name + return name, "" + } +} + +// boundedNumber is fallback moved inside whatever range the document +// declares, preferring a declared example that the same range admits. +// +// A constant is refused by the API when the property declares a minimum above +// it or a maximum below it, and that refusal names the constant rather than +// the field it came from. +func boundedNumber(a ir.Attribute, fallback float64) float64 { + value := fallback + if example, ok := numeric(a.Example); ok { + value = example + } + if a.Minimum != nil && value < *a.Minimum { + value = *a.Minimum + } + if a.Maximum != nil && value > *a.Maximum { + value = *a.Maximum + } + return value +} + +// numeric reads a document-declared number, which decodes as any of Go's +// numeric kinds depending on how it was spelled. +func numeric(value any) (float64, bool) { + switch n := value.(type) { + case float64: + return n, true + case float32: + return float64(n), true + case int: + return float64(n), true + case int64: + return float64(n), true + case uint64: + return float64(n), true } + return 0, false } // ValueForSDKType is the fixture value a generated SDK's own type demands, diff --git a/internal/fixtures/fixtures_test.go b/internal/fixtures/fixtures_test.go index d62be24..7df6919 100644 --- a/internal/fixtures/fixtures_test.go +++ b/internal/fixtures/fixtures_test.go @@ -406,3 +406,99 @@ func TestUnit_Fixturespec_AFormatDecidesTheValueShape(t *testing.T) { } } } + +// exampleTree is one tree exercising the declared-example precedence: a +// string the document describes only by example, one it also gives a format, +// one it constrains to an enum, and numbers the document bounds. +func exampleTree() *ir.AttributeTree { + minimum, maximum := 10.0, 1.0 + return &ir.AttributeTree{ + Attributes: []ir.Attribute{ + {Name: "name", WireName: "name", Kind: ir.TypeString, ComputedOptionalRequired: ir.Required, + Example: "Production metrics stream"}, + {Name: "endpoint_url", WireName: "endpointUrl", Kind: ir.TypeString, ComputedOptionalRequired: ir.Required, + Example: "https://api.example.otel-collector"}, + {Name: "created", WireName: "created", Kind: ir.TypeString, ComputedOptionalRequired: ir.Optional, + Format: "date-time", Example: "whenever"}, + {Name: "kind", WireName: "kind", Kind: ir.TypeString, ComputedOptionalRequired: ir.Optional, + OneOf: []string{"basic", "advanced"}, Example: "advanced"}, + {Name: "retries", WireName: "retries", Kind: ir.TypeInt64, ComputedOptionalRequired: ir.Optional, + Minimum: &minimum}, + {Name: "ratio", WireName: "ratio", Kind: ir.TypeFloat64, ComputedOptionalRequired: ir.Optional, + Maximum: &maximum}, + {Name: "interval", WireName: "interval", Kind: ir.TypeInt64, ComputedOptionalRequired: ir.Optional, + Example: 300}, + }, + } +} + +func TestUnit_Fixturespec_DeclaredExampleDisplacesTheInventedName(t *testing.T) { + s := Derive(exampleTree()) + + // A string the document describes only by example takes the example: an + // invented name is a string, and the API wanted a URL. + if got := valueByName(t, s, "endpoint_url").Scalar; got != "https://api.example.otel-collector" { + t.Errorf("endpoint_url = %#v, want the declared example", got) + } + // A declared format still wins: it synthesises a value of the right shape + // that keeps the prefix, which an example cannot. + if got := valueByName(t, s, "created").Scalar; got != "2026-01-02T03:04:05Z" { + t.Errorf("created = %#v, want the format-driven value", got) + } + // An enum still wins, being the stricter statement of the two. + if got := valueByName(t, s, "kind").Scalar; got != "basic" { + t.Errorf("kind = %#v, want the first enum value", got) + } +} + +func TestUnit_Fixturespec_DeclaredBoundsMoveTheNumericConstant(t *testing.T) { + s := Derive(exampleTree()) + + // The constant sits below the declared minimum, so the value moves up to + // it; a value the document forbids is refused by the API, not asserted on. + if got := valueByName(t, s, "retries").Scalar; got != int64(10) { + t.Errorf("retries = %#v, want the declared minimum", got) + } + if got := valueByName(t, s, "ratio").Scalar; got != 1.0 { + t.Errorf("ratio = %#v, want the declared maximum", got) + } + if got := valueByName(t, s, "interval").Scalar; got != int64(300) { + t.Errorf("interval = %#v, want the declared example", got) + } +} + +func TestUnit_Fixturespec_OneSynthesisedNameSurvivesForCleanup(t *testing.T) { + s := Derive(exampleTree()) + + // Every string in this tree could take an example, which would leave the + // created object carrying no prefix for the cleanup pass to match on. + if !anyPrefixed(s.Entries) { + t.Fatal("no derived string carries the name prefix") + } + // The first displaced name is the one restored, so the choice follows the + // document's order rather than the shape of any one field. + if got := valueByName(t, s, "name").Scalar; got != NamePrefix+"name" { + t.Errorf("name = %#v, want the restored synthesised name", got) + } + // Restoring one name does not take back any other example. + if got := valueByName(t, s, "endpoint_url").Scalar; got != "https://api.example.otel-collector" { + t.Errorf("endpoint_url = %#v, want the declared example kept", got) + } +} + +func TestUnit_Fixturespec_APrefixedStringSuppressesTheRestore(t *testing.T) { + tree := exampleTree() + // A string the document says nothing about already carries the prefix, so + // nothing needs restoring and every example stands. + tree.Attributes = append(tree.Attributes, ir.Attribute{ + Name: "label", WireName: "label", Kind: ir.TypeString, ComputedOptionalRequired: ir.Optional, + }) + s := Derive(tree) + + if got := valueByName(t, s, "label").Scalar; got != NamePrefix+"label" { + t.Fatalf("label = %#v, want the synthesised name", got) + } + if got := valueByName(t, s, "name").Scalar; got != "Production metrics stream" { + t.Errorf("name = %#v, want the declared example kept", got) + } +} diff --git a/internal/intermediate_representation/attributes.go b/internal/intermediate_representation/attributes.go index 774497e..549e2e7 100644 --- a/internal/intermediate_representation/attributes.go +++ b/internal/intermediate_representation/attributes.go @@ -36,7 +36,11 @@ type flat struct { // an attribute is inferred. description string enum []any - required map[string]bool + // example is the document's declared example value. It is the vendor's + // own statement of a value the API accepts, which is the only thing a + // document says about a string whose shape it otherwise leaves to prose. + example any + required map[string]bool // properties preserves encounter order — document order first, allOf // branches after — with the first declaration of a name winning. properties []specmodel.Property @@ -122,6 +126,9 @@ func flatten(schema *specmodel.Schema) flat { if flattened.enum == nil { flattened.enum = schema.Enum } + if flattened.example == nil { + flattened.example = schema.Example + } for _, name := range schema.Required { flattened.required[name] = true } @@ -456,6 +463,7 @@ func buildAttribute(wire string, attributeSite site) (Attribute, attributeEdges) // extra. writeOnly is the exception — only a request schema can declare // it, so a response-only attribute could never carry it anyway. attribute.Format = flatPrimary.format + attribute.Example = flatPrimary.example attribute.WriteOnly = flatCreate.writeOnly attribute.Deprecated = flatCreate.deprecated || flatRead.deprecated attribute.UniqueItems = flatPrimary.uniqueItems diff --git a/internal/intermediate_representation/constraints_test.go b/internal/intermediate_representation/constraints_test.go index 08eb92d..9edf7e2 100644 --- a/internal/intermediate_representation/constraints_test.go +++ b/internal/intermediate_representation/constraints_test.go @@ -328,3 +328,71 @@ func assertBound(t *testing.T, name string, got *int64, want int64) { t.Errorf("%s = %d, want %d", name, *got, want) } } + +// exampleSpec declares its example on a referenced schema rather than on the +// property, which is where a document that names its types puts it. +const exampleSpec = `openapi: 3.0.3 +info: {title: T, version: "1"} +paths: + /streams: + post: + requestBody: + content: + application/json: + schema: {$ref: '#/components/schemas/Stream'} + responses: + "201": + content: + application/json: + schema: {$ref: '#/components/schemas/Stream'} + /streams/{streamId}: + get: + responses: + "200": + content: + application/json: + schema: {$ref: '#/components/schemas/Stream'} + patch: + requestBody: + content: + application/json: + schema: {$ref: '#/components/schemas/Stream'} + responses: + "200": + content: + application/json: + schema: {$ref: '#/components/schemas/Stream'} + delete: + responses: + "204": {description: gone} +components: + schemas: + Stream: + type: object + properties: + endpointUrl: {$ref: '#/components/schemas/EndpointUrl'} + retries: {type: integer, format: int64, example: 300} + plain: {type: string} + EndpointUrl: + type: string + description: The URL data is sent to. + example: https://api.example.otel-collector +` + +// TestUnit_Attribute_CarriesTheDeclaredExample proves the example survives +// the reference it is declared behind, which is the only place the fixture +// derivation can learn that a string is more than a string when the document +// declares no format. +func TestUnit_Attribute_CarriesTheDeclaredExample(t *testing.T) { + r := resourceByKey(t, mustDerive(t, exampleSpec, testConfig()), "stream") + + if got := attribute(t, r.Schema, "endpoint_url").Example; got != "https://api.example.otel-collector" { + t.Errorf("Example = %#v, want the one declared on the referenced schema", got) + } + if got := attribute(t, r.Schema, "retries").Example; got != 300 { + t.Errorf("Example = %#v, want the declared number", got) + } + if got := attribute(t, r.Schema, "plain").Example; got != nil { + t.Errorf("a property declaring no example carries %#v", got) + } +} diff --git a/internal/intermediate_representation/model.go b/internal/intermediate_representation/model.go index 239da3c..a053321 100644 --- a/internal/intermediate_representation/model.go +++ b/internal/intermediate_representation/model.go @@ -324,6 +324,12 @@ type Attribute struct { // Format is the document's declared format, which says what a string // carries beyond being a string: "password", "date-time", "uuid". Format string `json:"format,omitempty"` + // Example is the document's declared example value. Fixture derivation + // prefers it to an invented value: a document that declares no format + // often still states, through an example, that the value has a shape the + // API enforces — a URL, a dotted identifier — and an invented string of + // the right type is refused by the API for the wrong reason. + Example any `json:"example,omitempty"` // WriteOnly marks a property the API accepts on write and never // returns. WriteOnly bool `json:"write_only,omitempty"` diff --git a/internal/sdkbind/model.go b/internal/sdkbind/model.go index 989dd84..b5df57f 100644 --- a/internal/sdkbind/model.go +++ b/internal/sdkbind/model.go @@ -127,6 +127,13 @@ type ResourceBinding struct { Read *Call `json:"read,omitempty"` Update *Call `json:"update,omitempty"` Delete *Call `json:"delete,omitempty"` + // CreateIDAccess is the accessor the create response answers the new + // object's id through, set only when that response is a different type + // from the read model and still carries the id. + // + // Without it the id is known only to the response the create discards, + // and the settling read addresses the object by an empty string. + CreateIDAccess string `json:"create_id_access,omitempty"` // ReadModel is the finished type expression fields are read from, // e.g. "models.Tagable" or "sdk.Tag". ReadModel string `json:"read_model,omitempty"` diff --git a/internal/sdkbind/prune.go b/internal/sdkbind/prune.go index 851304b..e9572ce 100644 --- a/internal/sdkbind/prune.go +++ b/internal/sdkbind/prune.go @@ -167,10 +167,52 @@ func (p *pruner) resource(rb *ResourceBinding) bool { return false } + p.settleCreateID(rb, read) p.settleUpdateBody(rb, read) return true } +// settleCreateID finds the accessor a create response answers the id through +// when that response is not the read model. +// +// A create that answers its own type still names the object it made; taking +// the id from it is what lets the settling read address the object at all. +func (p *pruner) settleCreateID(rb *ResourceBinding, read types.Type) { + if rb.Create == nil || rb.Create.ResponseType == "" || rb.Create.ResponseType == rb.ReadModel || read == nil { + return + } + var idAccess string + for i := range rb.Fields { + if rb.Fields[i].Attr == "id" { + idAccess = rb.Fields[i].Access.Get + break + } + } + if idAccess == "" { + return + } + created, err := p.resolveType(rb.Create.ResponseType) + if err != nil { + return + } + fromCreate, ok := methodOn(created, idAccess) + if !ok { + return + } + fromRead, ok := methodOn(read, idAccess) + if !ok { + return + } + // The same accessor on both, answering the same type: the id the state + // mapper already converts from the read is the id this takes from the + // create, so the conversion settled for one is right for the other. + if fromCreate.Results().Len() != 1 || fromRead.Results().Len() != 1 || + !types.Identical(fromCreate.Results().At(0).Type(), fromRead.Results().At(0).Type()) { + return + } + rb.CreateIDAccess = idAccess +} + // settleUpdateBody gives the update its own request body where the create's // cannot serve it. // diff --git a/internal/templates/services/datasource/responders.go.tmpl b/internal/templates/services/datasource/responders.go.tmpl index e40fd74..747a8b1 100644 --- a/internal/templates/services/datasource/responders.go.tmpl +++ b/internal/templates/services/datasource/responders.go.tmpl @@ -9,7 +9,7 @@ package mocks {{ .MocksImports }} // responseMaximal is the wire shape of one fully-populated object. -const responseMaximal = `{{ .ResponseMaximal }}` +const responseMaximal = {{ .ResponseMaximal }} {{ if .CollectionURL }} // collectionURL is where the list operation answers. diff --git a/internal/templates/services/resource/crud.go.tmpl b/internal/templates/services/resource/crud.go.tmpl index d461f07..7178c5b 100644 --- a/internal/templates/services/resource/crud.go.tmpl +++ b/internal/templates/services/resource/crud.go.tmpl @@ -51,6 +51,9 @@ func (r *{{ .Type }}) Create(ctx context.Context, req resource.CreateRequest, re return } {{ end }} +{{- if .CreateIDFromResponse }} + {{ .CreateIDFromResponse }} +{{- end }} {{- if .Singleton }} data.ID = types.StringValue({{ .SingletonID | printf "%q" }}) {{- end }} @@ -58,6 +61,11 @@ func (r *{{ .Type }}) Create(ctx context.Context, req resource.CreateRequest, re if resp.Diagnostics.HasError() { return } +{{ if .IdentitySets }} +{{ .IdentitySets }} if resp.Diagnostics.HasError() { + return + } +{{ end }} readReq := resource.ReadRequest{State: resp.State, ProviderMeta: req.ProviderMeta} opts := crud.ReadWithRetryOptions{Operation: errors.OperationCreate, ResourceTypeName: ResourceName} @@ -99,6 +107,11 @@ func (r *{{ .Type }}) Read(ctx context.Context, req resource.ReadRequest, resp * {{- if .Singleton }} data.ID = types.StringValue({{ .SingletonID | printf "%q" }}) {{- end }} +{{ if .IdentitySets }} +{{ .IdentitySets }} if resp.Diagnostics.HasError() { + return + } +{{ end }} resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) } diff --git a/internal/templates/services/resource/responders.go.tmpl b/internal/templates/services/resource/responders.go.tmpl index 3b0bd90..b766e59 100644 --- a/internal/templates/services/resource/responders.go.tmpl +++ b/internal/templates/services/resource/responders.go.tmpl @@ -18,7 +18,7 @@ var mockState = struct { // Wire fixtures, rendered from the same derivation as the terraform // fixtures so the two can never disagree. const ( - responseMinimal = `{{ .ResponseMinimal }}` + responseMinimal = {{ .ResponseMinimal }} ) // URLs this entity's operations answer on.