From 5ad1d995696fae8fbba4fe00dfabb691255ba99f Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:49:30 +0100 Subject: [PATCH] feat: read a list resource's path parameters from its list block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A collection path like /orgs/{org}/… had nowhere to read org from: the toolkit filled only the element schema, so emission refused the entity. A list resource now declares the addressing its collection path requires as the configuration of its list block, and reads it from req.Config. The derivation reuses ensureParentParameters, which the resource and datasource paths already call; every parameter is a parent, because a collection path carries no item key. None carries RequiresReplace: a list block declares a query, and a query has no plan to modify. A failed path-parameter conversion now reports through the method it lands in rather than assuming one: every lifecycle and invoke method carries a resp with Diagnostics, and List carries a results stream. schemaBuilder gains the list/schema package and the fact that it, like the action package, declares no Computed. Co-Authored-By: Claude Opus 5 (1M context) --- internal/emit/render_action.go | 2 +- internal/emit/render_datasource.go | 6 +- internal/emit/render_listresource.go | 58 ++++++-- internal/emit/render_listresource_test.go | 124 ++++++++++++++++++ internal/emit/render_mapping.go | 75 ++++++++--- internal/emit/render_mapping_test.go | 8 +- internal/emit/render_resource.go | 8 +- internal/emit/render_schema.go | 49 ++++--- internal/emit/services_errors_test.go | 3 +- .../intermediate_representation/attributes.go | 20 +++ .../intermediate_representation/derive.go | 10 +- .../derive_test.go | 31 +++++ internal/intermediate_representation/model.go | 4 + .../services/list-resource/list.go.tmpl | 8 ++ .../list-resource/list_resource.go.tmpl | 7 +- .../list-resource/list_resource_test.go.tmpl | 27 +++- .../services/list-resource/model.go.tmpl | 4 + 17 files changed, 381 insertions(+), 63 deletions(-) create mode 100644 internal/emit/render_listresource_test.go diff --git a/internal/emit/render_action.go b/internal/emit/render_action.go index e16688a..8e425c9 100644 --- a/internal/emit/render_action.go +++ b/internal/emit/render_action.go @@ -113,7 +113,7 @@ func (e *serviceRenderer) action(a *ir.Action, ab *sdkbind.ActionBinding) ([]Fil } } - plan, err := buildCallPlan(ab.Invoke, "", nodes, "data") + plan, err := buildCallPlan(ab.Invoke, "", nodes, "data", respDiagnostics()) if err != nil { return nil, fmt.Errorf("invoke: %w", err) } diff --git a/internal/emit/render_datasource.go b/internal/emit/render_datasource.go index 86a31af..ff031d0 100644 --- a/internal/emit/render_datasource.go +++ b/internal/emit/render_datasource.go @@ -167,7 +167,7 @@ func (e *serviceRenderer) lookupDatasource(d *datasourceData, ds *ir.Datasource, d.Models = renderModelDecls(decls) d.ModelImports = e.datasourceModelImports(d.Models) - plan, err := buildCallPlan(db.Read, "remote", nodes, "data") + plan, err := buildCallPlan(db.Read, "remote", nodes, "data", respDiagnostics()) if err != nil { return fixtures.Fixture{}, fmt.Errorf("read: %w", err) } @@ -304,7 +304,7 @@ func (e *serviceRenderer) companionDatasource(d *datasourceData, ds *ir.Datasour for _, a := range companionAddressing(ds) { addressingNodes = append(addressingNodes, node{attr: a}) } - listPlan, err := buildCallPlan(db.List, "result", addressingNodes, "data") + listPlan, err := buildCallPlan(db.List, "result", addressingNodes, "data", respDiagnostics()) if err != nil { return fixtures.Fixture{}, fmt.Errorf("list: %w", err) } @@ -384,7 +384,7 @@ func itemPayloadExpr(db *sdkbind.DatasourceBinding) string { func readPlanWithoutParams(call *sdkbind.Call, payloadName string) (callPlan, error) { stripped := *call stripped.Params = nil - return buildCallPlan(&stripped, payloadName, nil, "data") + return buildCallPlan(&stripped, payloadName, nil, "data", respDiagnostics()) } // companionItemTree finds the items attribute's element tree. diff --git a/internal/emit/render_listresource.go b/internal/emit/render_listresource.go index 75a2034..ba4ceaf 100644 --- a/internal/emit/render_listresource.go +++ b/internal/emit/render_listresource.go @@ -28,18 +28,28 @@ type listResourceData struct { TestImports string SchemaDescription string + SchemaAttributes string + ConfigModel string ListPlan callPlan Collection string ResultLines string - CollectionURL string + CollectionURL string + CollectionPattern string + ListResponse string ExpectedFirstID string + ConfigValue string TestClientConfig string AuthGitHubApp bool ProviderName string } +// listConfigModelName is the struct the list block's configuration decodes +// into, and the local List reads it as. Unexported and per-package, so it +// needs no entity prefix. +const listConfigModelName = "listConfigModel" + // listResource renders one list-only entity's file set. func (e *serviceRenderer) listResource(lr *ir.ListResource, lb *sdkbind.ListResourceBinding) ([]File, error) { if lb.List == nil { @@ -65,17 +75,20 @@ func (e *serviceRenderer) listResource(lr *ir.ListResource, lb *sdkbind.ListReso } d.SchemaDescription = strconv.Quote(description) + // The call's path parameters are read from the list block's own + // configuration, not from an element: a list resource has no object to + // address, so the practitioner supplies the scope the collection path + // names. + configNodes := joinTree(lr.AddressingSchema, nil, addressingNames(&lr.ListOperation)) + listOp := lr.ListOperation - plan, err := buildCallPlan(lb.List, "result", nodes, "data") + plan, err := buildCallPlan(lb.List, "result", configNodes, "config", streamDiagnostics()) if err != nil { return nil, fmt.Errorf("list: %w", err) } if plan.Payload == "" { return nil, unrenderable("list: the bound list call yields no payload") } - if plan.ParamDecls != "" { - return nil, unrenderable("list: a list resource cannot supply path parameters; the call needs %q", lb.List.Params[0].Wire) - } d.ListPlan = plan d.Collection = "result" if lb.CollectionAccess != "" { @@ -95,8 +108,14 @@ func (e *serviceRenderer) listResource(lr *ir.ListResource, lb *sdkbind.ListReso imports.add("listschema", "github.com/hashicorp/terraform-plugin-framework/list/schema") imports.add("", "github.com/hashicorp/terraform-plugin-framework/resource") imports.add("sdk", e.bindings.SDK.ImportPath) + sb := &schemaBuilder{kind: schemaListResource, imports: imports} + d.SchemaAttributes = sb.attributeDecls(configNodes, 3) d.Imports = imports.render() + if len(configNodes) > 0 { + d.ConfigModel = renderModelDecls(buildModels(listConfigModelName, lr.Names.Pascal+"ListConfig", configNodes, nil)) + } + listImports := newImportSet(e.pc.Module) listImports.add("", "context") listImports.add("", "github.com/hashicorp/terraform-plugin-framework/diag") @@ -110,7 +129,18 @@ func (e *serviceRenderer) listResource(lr *ir.ListResource, lb *sdkbind.ListReso d.ListImports = listImports.render() spec := deriveFixtures(lr.Schema, nodes) - d.CollectionURL = mockURL(listOp.PathTemplate) + configSpec := deriveFixtures(lr.AddressingSchema, configNodes) + // A parameterised collection path is requested with the addressing + // substituted in, so the mock matches the shape rather than the template, + // and the unit test stands a configuration up to be read from. The + // pattern is a regex and travels separately: it carries backslashes a + // quoted Go string cannot hold. + if len(configNodes) > 0 { + d.CollectionPattern = mockPattern(listOp.PathTemplate) + d.ConfigValue = tftypesValue(configSpec.Entries, 1) + } else { + d.CollectionURL = mockURL(listOp.PathTemplate) + } item := strings.TrimSuffix(string(spec.WireJSON(fixtures.ResponseMaximal)), "\n") d.ListResponse = listResponseJSON(lr.ListEnvelopeKey, item) d.ExpectedFirstID = expectedID(spec) @@ -125,6 +155,10 @@ func (e *serviceRenderer) listResource(lr *ir.ListResource, lb *sdkbind.ListReso testImports.add("", "github.com/hashicorp/terraform-plugin-framework/resource") testImports.add("identityschema", "github.com/hashicorp/terraform-plugin-framework/resource/identityschema") testImports.add("", "github.com/hashicorp/terraform-plugin-framework/types") + if d.ConfigValue != "" { + testImports.add("", "github.com/hashicorp/terraform-plugin-framework/tfsdk") + testImports.add("", "github.com/hashicorp/terraform-plugin-go/tftypes") + } testImports.add("", "github.com/jarcoal/httpmock") testImports.add("", e.pc.Module+"/internal/client") testImports.add("", e.pc.Module+"/internal/mocks") @@ -155,7 +189,8 @@ func (e *serviceRenderer) listResource(lr *ir.ListResource, lb *sdkbind.ListReso files = append(files, rawFile(path.Join(dir, "tests/responses/list.json"), lr.Names.Key, []byte(d.ListResponse))) - example, err := listExample(lr.Names.Key, lr.Names.TerraformType, e.pc.ProviderName) + example, err := listExample(lr.Names.Key, lr.Names.TerraformType, e.pc.ProviderName, + configSpec.HCL(fixtures.ConfigMinimal)) if err != nil { return nil, err } @@ -268,12 +303,15 @@ func (e *serviceRenderer) testClientConfig() string { } } -// listExample renders the terraform query example. -func listExample(source, terraformType, providerName string) ([]byte, error) { +// listExample renders the terraform query example. configBody is the +// addressing the list block requires, already rendered as HCL assignments, +// empty for a collection path that takes no parameters. +func listExample(source, terraformType, providerName, configBody string) ([]byte, error) { header, err := hashHeader(source) if err != nil { return nil, err } - body := fmt.Sprintf("%s\nlist %q \"example\" {\n provider = %s\n}\n", header, terraformType, providerName) + body := fmt.Sprintf("%s\nlist %q \"example\" {\n provider = %s\n%s}\n", + header, terraformType, providerName, configBody) return []byte(body), nil } diff --git a/internal/emit/render_listresource_test.go b/internal/emit/render_listresource_test.go new file mode 100644 index 0000000..14d9e9a --- /dev/null +++ b/internal/emit/render_listresource_test.go @@ -0,0 +1,124 @@ +package emit + +import ( + "strings" + "testing" + + ir "github.com/deploymenttheory/terraform-plugin-framework-codegen/internal/intermediate_representation" + "github.com/deploymenttheory/terraform-plugin-framework-codegen/internal/sdkbind" +) + +// scopedListResource is the fictional tree with its list resource moved +// behind a parent-scoped collection path, which is the shape most of a +// parent-scoped document takes. +func scopedListResource(t *testing.T) *ServiceFiles { + t.Helper() + m, b := fictionalModel(), fictionalBindings() + m.ListResources[0].ListOperation.PathTemplate = "/v7/tenants/{tenantId}/audit-events" + m.ListResources[0].ListOperation.PathParameters = []ir.Parameter{{Name: "tenantId", Type: ir.TypeString}} + m.ListResources[0].AddressingSchema = &ir.AttributeTree{Attributes: []ir.Attribute{ + {Name: "tenant_id", WireName: "tenantId", Kind: ir.TypeString, ComputedOptionalRequired: ir.Required}, + }} + b.ListResources["audit_event"].List.Params = []sdkbind.CallParam{ + {Local: "tenantId", GoType: "string", Wire: "tenantId"}} + + out, err := RenderServices(fictionalProviderCore(), m, b) + if err != nil { + t.Fatalf("a parent-scoped list resource must render: %v", err) + } + return out +} + +// TestUnit_ListResource_ReadsItsPathParametersFromTheListBlock proves a +// collection path's parameters are declared as the list block's own +// configuration and read from there, rather than refusing the entity. +func TestUnit_ListResource_ReadsItsPathParametersFromTheListBlock(t *testing.T) { + out := scopedListResource(t) + dir := "internal/services/list-resources/audit/v7/audit_event/" + + schema := string(fileByPath(t, out, dir+"list_resource.go").Content) + for _, want := range []string{ + "Attributes: map[string]listschema.Attribute{", + `"tenant_id": listschema.StringAttribute{`, + "Required:", + } { + if !strings.Contains(schema, want) { + t.Errorf("the config schema does not carry %q:\n%s", want, schema) + } + } + + model := string(fileByPath(t, out, dir+"model.go").Content) + if !strings.Contains(model, "type listConfigModel struct {") || + !strings.Contains(model, "TenantID types.String `tfsdk:\"tenant_id\"`") { + t.Errorf("model.go does not declare the config model:\n%s", model) + } + + list := string(fileByPath(t, out, dir+"list.go").Content) + for _, want := range []string{ + "var config listConfigModel", + "req.Config.Get(ctx, &config)", + "tenantId := config.TenantID.ValueString()", + } { + if !strings.Contains(list, want) { + t.Errorf("List does not read its addressing from the configuration, missing %q:\n%s", want, list) + } + } +} + +// TestUnit_ListResource_MocksAParameterisedPathByShape proves the generated +// unit test matches the request by pattern, because a parameterised path is +// requested with the addressing substituted in rather than as the template, +// and that it stands a configuration up for List to read. +func TestUnit_ListResource_MocksAParameterisedPathByShape(t *testing.T) { + out := scopedListResource(t) + test := string(fileByPath(t, out, + "internal/services/list-resources/audit/v7/audit_event/list_resource_test.go").Content) + + for _, want := range []string{ + "httpmock.RegisterResponder(\"GET\", `=~^", + `/v7/tenants/([^/]+)/audit-events$`, + "func listConfig(t *testing.T, lr list.ListResource) tfsdk.Config {", + "listConfig(t, lr),", + } { + if !strings.Contains(test, want) { + t.Errorf("the generated test does not carry %q:\n%s", want, test) + } + } + if strings.Contains(test, `"{{`) { + t.Errorf("the generated test carries an unrendered action:\n%s", test) + } +} + +// TestUnit_ListResource_ExampleSuppliesTheRequiredAddressing proves the +// emitted query example sets the attributes the list block requires, so it +// is a configuration terraform would accept rather than one it would reject. +func TestUnit_ListResource_ExampleSuppliesTheRequiredAddressing(t *testing.T) { + out := scopedListResource(t) + example := string(fileByPath(t, out, + "examples/list-resources/petstore_audit_event/list-resource.tfquery.hcl").Content) + + if !strings.Contains(example, "tenant_id = ") { + t.Errorf("the example does not supply the required addressing:\n%s", example) + } +} + +// TestUnit_ListResource_WithoutAddressingDeclaresNoConfiguration proves a +// collection path that takes no parameters is unchanged: an empty list +// block, no config model, and the mock matched by exact URL. +func TestUnit_ListResource_WithoutAddressingDeclaresNoConfiguration(t *testing.T) { + out := renderFictional(t) + dir := "internal/services/list-resources/audit/v7/audit_event/" + + if schema := string(fileByPath(t, out, dir+"list_resource.go").Content); strings.Contains(schema, "Attributes: map[string]listschema.Attribute{") { + t.Errorf("an unparameterised collection path must declare an empty list block:\n%s", schema) + } + if model := string(fileByPath(t, out, dir+"model.go").Content); strings.Contains(model, "listConfigModel") { + t.Errorf("an unparameterised collection path needs no config model:\n%s", model) + } + if list := string(fileByPath(t, out, dir+"list.go").Content); strings.Contains(list, "req.Config.Get") { + t.Errorf("an unparameterised collection path reads no configuration:\n%s", list) + } + if test := string(fileByPath(t, out, dir+"list_resource_test.go").Content); !strings.Contains(test, `httpmock.RegisterResponder("GET", "https://unit.invalid/v7/audit-events"`) { + t.Errorf("an unparameterised collection path is mocked by exact URL:\n%s", test) + } +} diff --git a/internal/emit/render_mapping.go b/internal/emit/render_mapping.go index 37cf134..6daf6e0 100644 --- a/internal/emit/render_mapping.go +++ b/internal/emit/render_mapping.go @@ -312,9 +312,52 @@ type callPlan struct { Imports []string } +// paramFailure is what a path-parameter conversion that fails does in the +// method its declaration lands in. Every lifecycle and invoke method carries +// a resp with Diagnostics; a list resource carries a results stream instead, +// and the two report an error by different names. +type paramFailure struct { + // report renders the statements a failed conversion runs, given the + // terraform attribute at fault, the parameter's wire name, and the local + // holding the error. It ends by leaving the method. + report func(attribute, wire, errLocal string) string + // imports names the packages report's statements reference. + imports []string +} + +// respDiagnostics reports a failed conversion against the attribute at fault. +// Every method a declaration lands in — Create, Read, Update, Delete, Invoke — +// carries a resp with Diagnostics and returns nothing. +func respDiagnostics() paramFailure { + return paramFailure{ + report: func(attribute, wire, errLocal string) string { + return fmt.Sprintf("\tresp.Diagnostics.AddAttributeError(path.Root(%q), \"Invalid %s\", %s.Error())\n\t\treturn", + attribute, wire, errLocal) + }, + imports: []string{"github.com/hashicorp/terraform-plugin-framework/path"}, + } +} + +// streamDiagnostics reports a failed conversion into a list resource's +// results stream, which is the only channel List has: it takes no resp, and +// a stream carrying diagnostics is how the framework surfaces the failure. +func streamDiagnostics() paramFailure { + return paramFailure{ + report: func(attribute, wire, errLocal string) string { + return fmt.Sprintf("\tstream.Results = list.ListResultsStreamDiagnostics(diag.Diagnostics{\n\t\t\tdiag.NewErrorDiagnostic(\"Invalid %s\", %s.Error()),\n\t\t})\n\t\treturn", + wire, errLocal) + }, + imports: []string{ + "github.com/hashicorp/terraform-plugin-framework/diag", + "github.com/hashicorp/terraform-plugin-framework/list", + }, + } +} + // buildCallPlan renders one bound call. payloadName names the success -// payload local; nodes and modelVar say where parameter values come from. -func buildCallPlan(call *sdkbind.Call, payloadName string, nodes []node, modelVar string) (callPlan, error) { +// payload local; nodes and modelVar say where parameter values come from; +// fail says how a conversion that cannot succeed reports itself. +func buildCallPlan(call *sdkbind.Call, payloadName string, nodes []node, modelVar string, fail paramFailure) (callPlan, error) { var plan callPlan var decls []string @@ -330,7 +373,7 @@ func buildCallPlan(call *sdkbind.Call, payloadName string, nodes []node, modelVa if err != nil { return callPlan{}, err } - decl, needs, err := paramDeclaration(p, modelVar, ir.GoName(n.attr.Name), n.attr.Kind, n.attr.Name) + decl, needs, err := paramDeclaration(p, modelVar, ir.GoName(n.attr.Name), n.attr.Kind, n.attr.Name, fail) if err != nil { return callPlan{}, err } @@ -474,25 +517,22 @@ func paramValue(p sdkbind.CallParam, modelVar, field string, kind ir.AttributeTy // paramDeclaration renders the statements that bind one path parameter's // local: an assignment for a conversion that cannot fail, and a parse -// guarded by an attribute diagnostic for one that can. -// -// Every method a declaration lands in — Create, Read, Update, Delete, -// Invoke — carries a resp with Diagnostics and returns nothing, so a -// failed parse reports against the attribute and stops there. -func paramDeclaration(p sdkbind.CallParam, modelVar, field string, kind ir.AttributeType, attribute string) (string, []string, error) { +// guarded by a diagnostic for one that can. fail says how that diagnostic is +// reported in the method the declaration lands in. +func paramDeclaration(p sdkbind.CallParam, modelVar, field string, kind ir.AttributeType, attribute string, fail paramFailure) (string, []string, error) { if kind == ir.TypeString { read := modelVar + "." + field + ".ValueString()" switch { case p.GoType == "uuid.UUID": - return guardedParse(p, "uuid.Parse("+read+")", "", attribute), - []string{"github.com/google/uuid", "github.com/hashicorp/terraform-plugin-framework/path"}, nil + return guardedParse(p, "uuid.Parse("+read+")", "", attribute, fail), + append([]string{"github.com/google/uuid"}, fail.imports...), nil case isIntegerType(p.GoType): // The parse is sized to the SDK's own width, so a value the // parameter cannot hold is reported against the attribute rather // than wrapping in a cast. parse, cast := integerParse(p, read) - return guardedParse(p, parse, cast, attribute), - []string{"strconv", "github.com/hashicorp/terraform-plugin-framework/path"}, nil + return guardedParse(p, parse, cast, attribute, fail), + append([]string{"strconv"}, fail.imports...), nil } } @@ -514,18 +554,15 @@ func paramDeclaration(p sdkbind.CallParam, modelVar, field string, kind ir.Attri // parse must be a two-value expression yielding a value and an error. When // cast is empty the parsed value is already the local, so the parse binds it // directly; otherwise the parse binds an intermediate the cast reads. -func guardedParse(p sdkbind.CallParam, parse, cast, attribute string) string { +func guardedParse(p sdkbind.CallParam, parse, cast, attribute string, fail paramFailure) string { bound, tail := p.Local, "" if cast != "" { bound = p.Local + "Parsed" tail = "\n\t" + p.Local + " := " + cast } errLocal := p.Local + "Err" - return fmt.Sprintf(`%s, %s := %s - if %s != nil { - resp.Diagnostics.AddAttributeError(path.Root(%q), "Invalid %s", %s.Error()) - return - }%s`, bound, errLocal, parse, errLocal, attribute, p.Wire, errLocal, tail) + return fmt.Sprintf("%s, %s := %s\n\tif %s != nil {\n%s\n\t}%s", + bound, errLocal, parse, errLocal, fail.report(attribute, p.Wire, errLocal), tail) } // integerParse renders the strconv call that reads one integer path parameter diff --git a/internal/emit/render_mapping_test.go b/internal/emit/render_mapping_test.go index d326e9d..c5dce40 100644 --- a/internal/emit/render_mapping_test.go +++ b/internal/emit/render_mapping_test.go @@ -14,7 +14,7 @@ import ( func TestUnit_ParamDeclaration_UUIDParsesWithADiagnostic(t *testing.T) { p := sdkbind.CallParam{Local: "agentId", Wire: "agentId", GoType: "uuid.UUID"} - decl, imports, err := paramDeclaration(p, "data", "AgentID", ir.TypeString, "agent_id") + decl, imports, err := paramDeclaration(p, "data", "AgentID", ir.TypeString, "agent_id", respDiagnostics()) if err != nil { t.Fatalf("paramDeclaration refused a uuid path parameter: %v", err) } @@ -82,7 +82,7 @@ func TestUnit_ParamDeclaration_IntegerParsesWithADiagnostic(t *testing.T) { }, }, } { - decl, imports, err := paramDeclaration(testCase.param, "data", "HookID", ir.TypeString, "hook_id") + decl, imports, err := paramDeclaration(testCase.param, "data", "HookID", ir.TypeString, "hook_id", respDiagnostics()) if err != nil { t.Errorf("%s: paramDeclaration refused an integer path parameter: %v", testCase.name, err) continue @@ -115,7 +115,7 @@ func TestUnit_ParamDeclaration_IntegerParsesWithADiagnostic(t *testing.T) { func TestUnit_ParamDeclaration_RefusesATruncatingConversion(t *testing.T) { p := sdkbind.CallParam{Local: "groupId", Wire: "runner_group_id", GoType: "int32"} - if _, _, err := paramDeclaration(p, "data", "GroupID", ir.TypeFloat64, "runner_group_id"); err == nil { + if _, _, err := paramDeclaration(p, "data", "GroupID", ir.TypeFloat64, "runner_group_id", respDiagnostics()); err == nil { t.Fatal("paramDeclaration rendered a float64 into an int32 parameter; it must refuse") } } @@ -135,7 +135,7 @@ func TestUnit_ParamDeclaration_InfallibleConversionsStayOneLine(t *testing.T) { {"int64 narrowed", sdkbind.CallParam{Local: "id", Wire: "id", GoType: "int32"}, ir.TypeInt64, "id := int32(data.ID.ValueInt64())", ""}, {"int64 to string", sdkbind.CallParam{Local: "id", Wire: "id", GoType: "string"}, ir.TypeInt64, "id := strconv.FormatInt(data.ID.ValueInt64(), 10)", "strconv"}, } { - decl, imports, err := paramDeclaration(testCase.param, "data", "ID", testCase.kind, "id") + decl, imports, err := paramDeclaration(testCase.param, "data", "ID", testCase.kind, "id", respDiagnostics()) if err != nil { t.Errorf("%s: %v", testCase.name, err) continue diff --git a/internal/emit/render_resource.go b/internal/emit/render_resource.go index c0e839e..358017c 100644 --- a/internal/emit/render_resource.go +++ b/internal/emit/render_resource.go @@ -352,17 +352,17 @@ func (e *serviceRenderer) resourceCRUD(d *resourceData, rb *sdkbind.ResourceBind if d.CreateMapsResponse { createPayload = "created" } - if d.CreatePlan, err = buildCallPlan(createCall, createPayload, nodes, "data"); err != nil { + if d.CreatePlan, err = buildCallPlan(createCall, createPayload, nodes, "data", respDiagnostics()); err != nil { return fmt.Errorf("create: %w", err) } - if d.ReadPlan, err = buildCallPlan(rb.Read, "remote", nodes, "data"); err != nil { + if d.ReadPlan, err = buildCallPlan(rb.Read, "remote", nodes, "data", respDiagnostics()); err != nil { return fmt.Errorf("read: %w", err) } if d.ReadPlan.Payload == "" { return unrenderable("read: the bound read call yields no payload to map state from") } if !d.Singleton { - if d.DeletePlan, err = buildCallPlan(rb.Delete, "", nodes, "data"); err != nil { + if d.DeletePlan, err = buildCallPlan(rb.Delete, "", nodes, "data", respDiagnostics()); err != nil { return fmt.Errorf("delete: %w", err) } } @@ -372,7 +372,7 @@ func (e *serviceRenderer) resourceCRUD(d *resourceData, rb *sdkbind.ResourceBind if d.UpdateMapsResponse { updatePayload = "updated" } - if d.UpdatePlan, err = buildCallPlan(rb.Update, updatePayload, nodes, "prior"); err != nil { + if d.UpdatePlan, err = buildCallPlan(rb.Update, updatePayload, nodes, "prior", respDiagnostics()); err != nil { return fmt.Errorf("update: %w", err) } var copies []string diff --git a/internal/emit/render_schema.go b/internal/emit/render_schema.go index 661cd5d..acf040a 100644 --- a/internal/emit/render_schema.go +++ b/internal/emit/render_schema.go @@ -10,16 +10,37 @@ import ( ) // schemaKind selects which terraform-plugin-framework schema package a -// declaration is rendered against. The three packages spell the same -// shapes, but only the resource one carries plan modifiers. +// declaration is rendered against. The four packages spell the same +// shapes, but only the resource one carries plan modifiers, and only the +// resource and datasource ones carry Computed. type schemaKind int const ( schemaResource schemaKind = iota schemaDatasource schemaAction + schemaListResource ) +// pkg is the package name a schema declaration qualifies its attribute types +// with. Three of the four packages are imported under the framework's own +// name; list/schema is imported as listschema, because a list resource file +// also names the resource schema package. +func (sb *schemaBuilder) pkg() string { + if sb.kind == schemaListResource { + return "listschema" + } + return "schema" +} + +// rendersComputed reports whether the schema package declares Computed at +// all. An action's does not — an invocation has arguments and a result and +// nothing in between for the framework to fill in — and a list resource's +// does not either: every list/schema attribute answers false from IsComputed. +func (sb *schemaBuilder) rendersComputed() bool { + return sb.kind == schemaResource || sb.kind == schemaDatasource +} + // schemaBuilder accumulates the imports one schema declaration needs as // it renders. type schemaBuilder struct { @@ -50,7 +71,7 @@ func (sb *schemaBuilder) attributeDecl(n node, depth int) string { indent := strings.Repeat("\t", depth) var b strings.Builder - fmt.Fprintf(&b, "%s%q: schema.%s{\n", indent, n.attr.Name, schemaTypeOf(n).SchemaAttribute) + fmt.Fprintf(&b, "%s%q: %s.%s{\n", indent, n.attr.Name, sb.pkg(), schemaTypeOf(n).SchemaAttribute) b.WriteString(sb.computedOptionalRequiredLines(n, indent+"\t")) if desc := attributeDescription(n.attr); desc != "" { @@ -68,12 +89,12 @@ func (sb *schemaBuilder) attributeDecl(n node, depth int) string { if n.attr.Nested != nil { if n.attr.Kind == ir.TypeList { - fmt.Fprintf(&b, "%s\tNestedObject: schema.NestedAttributeObject{\n", indent) - fmt.Fprintf(&b, "%s\t\tAttributes: map[string]schema.Attribute{\n", indent) + fmt.Fprintf(&b, "%s\tNestedObject: %s.NestedAttributeObject{\n", indent, sb.pkg()) + fmt.Fprintf(&b, "%s\t\tAttributes: map[string]%s.Attribute{\n", indent, sb.pkg()) b.WriteString(sb.attributeDecls(n.children, depth+3)) fmt.Fprintf(&b, "%s\t\t},\n%s\t},\n", indent, indent) } else { - fmt.Fprintf(&b, "%s\tAttributes: map[string]schema.Attribute{\n", indent) + fmt.Fprintf(&b, "%s\tAttributes: map[string]%s.Attribute{\n", indent, sb.pkg()) b.WriteString(sb.attributeDecls(n.children, depth+2)) fmt.Fprintf(&b, "%s\t},\n", indent) } @@ -147,24 +168,22 @@ func (sb *schemaBuilder) validatorLines(n node, indent string, depth int) string indent, schemaTypeOf(n).Validator, strings.Join(definitions, ", ")) } -// computedOptionalRequiredLines renders the presence booleans. Inside a datasource, computed -// stays computed. Inside an action there is no Computed to render: the -// action package's attribute types do not declare the field, because an -// invocation has arguments and a result and nothing in between for the -// framework to fill in. An attribute that is writable as well keeps the -// writable half; one that is only computed is dropped before it reaches -// here. +// computedOptionalRequiredLines renders the presence booleans. Inside a +// datasource, computed stays computed. Where the schema package declares no +// Computed at all — see rendersComputed — an attribute that is writable as +// well keeps the writable half; one that is only computed is dropped before +// it reaches here. func (sb *schemaBuilder) computedOptionalRequiredLines(n node, indent string) string { switch n.attr.ComputedOptionalRequired { case ir.Required: return indent + "Required: true,\n" case ir.Computed: - if sb.kind == schemaAction { + if !sb.rendersComputed() { return indent + "Optional: true,\n" } return indent + "Computed: true,\n" case ir.ComputedOptional: - if sb.kind == schemaAction { + if !sb.rendersComputed() { return indent + "Optional: true,\n" } return indent + "Optional: true,\n" + indent + "Computed: true,\n" diff --git a/internal/emit/services_errors_test.go b/internal/emit/services_errors_test.go index 40887a5..2576307 100644 --- a/internal/emit/services_errors_test.go +++ b/internal/emit/services_errors_test.go @@ -226,7 +226,8 @@ func TestUnit_RenderServices_NamesTheEntityAndAttributeAtFault(t *testing.T) { b.ListResources["audit_event"].Fields = b.ListResources["audit_event"].Fields[1:] expectRenderExclusion(t, pc, m, b, "audit_event", "id") - // A list resource whose call demands a path parameter. + // A list resource whose call demands a path parameter no addressing + // attribute answers. m, b = fictionalModel(), fictionalBindings() b.ListResources["audit_event"].List.Params = []sdkbind.CallParam{ {Local: "parentId", GoType: "string", Wire: "parentId"}} diff --git a/internal/intermediate_representation/attributes.go b/internal/intermediate_representation/attributes.go index 75c5210..3e78c50 100644 --- a/internal/intermediate_representation/attributes.go +++ b/internal/intermediate_representation/attributes.go @@ -682,6 +682,26 @@ func ensureParentParameters(tree *AttributeTree, parents []Parameter) { tree.Attributes = append(added, tree.Attributes...) } +// addressingSchema is a collection path's addressing attributes as a tree of +// their own, for a list resource to declare as the configuration of its list +// block. Nil when the path takes no parameters. +// +// Every parameter is a parent: a collection path carries no item key, so +// there is no id to absorb the last one. None carries RequiresReplace — a +// list block declares a query, and a query has no plan for a modifier to act +// on. +func addressingSchema(parameters []Parameter) *AttributeTree { + if len(parameters) == 0 { + return nil + } + tree := &AttributeTree{} + ensureParentParameters(tree, parameters) + for index := range tree.Attributes { + tree.Attributes[index].RequiresReplace = false + } + return tree +} + // parentParameters is an operation's path parameters above the item key: all // of them but the last, which addresses the object itself and becomes the id. func parentParameters(parameters []Parameter) []Parameter { diff --git a/internal/intermediate_representation/derive.go b/internal/intermediate_representation/derive.go index 0b94f42..8d3baac 100644 --- a/internal/intermediate_representation/derive.go +++ b/internal/intermediate_representation/derive.go @@ -440,11 +440,13 @@ func (derivation *deriver) datasource(classification specmodel.Classification, n func (derivation *deriver) listResource(classification specmodel.Classification, names Names) ListResource { listFull := derivation.full(classification.List) element := listElementSchema(listFull) + listOperation := *derivation.operation(classification.List, OperationList) return ListResource{ - Names: names, - ListOperation: *derivation.operation(classification.List, OperationList), - Schema: buildTree(nil, element, nil, false), - ListEnvelopeKey: listEnvelopeKey(listFull), + Names: names, + ListOperation: listOperation, + Schema: buildTree(nil, element, nil, false), + AddressingSchema: addressingSchema(listOperation.PathParameters), + ListEnvelopeKey: listEnvelopeKey(listFull), } } diff --git a/internal/intermediate_representation/derive_test.go b/internal/intermediate_representation/derive_test.go index edd37a8..4f7654f 100644 --- a/internal/intermediate_representation/derive_test.go +++ b/internal/intermediate_representation/derive_test.go @@ -307,6 +307,37 @@ func TestDerive_ListResource(t *testing.T) { t.Fatalf("no event list resource in %+v", m.ListResources) } +// TestUnit_AddressingSchema_TakesEveryPathParameter proves a collection +// path's parameters all become required attributes of the list block: a +// collection path carries no item key, so none of them is absorbed by an id, +// and none carries RequiresReplace because a list block has no plan. +func TestUnit_AddressingSchema_TakesEveryPathParameter(t *testing.T) { + if tree := addressingSchema(nil); tree != nil { + t.Errorf("a path with no parameters declares no configuration, got %+v", tree) + } + + tree := addressingSchema([]Parameter{ + {Name: "tenantId", Type: TypeString}, + {Name: "groupId", Type: TypeInt64}, + }) + if tree == nil || len(tree.Attributes) != 2 { + t.Fatalf("addressingSchema = %+v", tree) + } + for index, want := range []Attribute{ + {Name: "tenant_id", WireName: "tenantId", Kind: TypeString, ComputedOptionalRequired: Required}, + {Name: "group_id", WireName: "groupId", Kind: TypeInt64, ComputedOptionalRequired: Required}, + } { + got := tree.Attributes[index] + if got.Name != want.Name || got.WireName != want.WireName || got.Kind != want.Kind || + got.ComputedOptionalRequired != want.ComputedOptionalRequired { + t.Errorf("attribute %d = %+v, want %+v", index, got, want) + } + if got.RequiresReplace { + t.Errorf("attribute %d carries RequiresReplace; a list block has no plan to modify", index) + } + } +} + func TestDerive_Action(t *testing.T) { m := mustDerive(t, thingSpec, testConfig()) if len(m.Actions) != 1 { diff --git a/internal/intermediate_representation/model.go b/internal/intermediate_representation/model.go index b830703..9abc4af 100644 --- a/internal/intermediate_representation/model.go +++ b/internal/intermediate_representation/model.go @@ -137,6 +137,10 @@ type ListResource struct { ListOperation Operation `json:"list_operation"` // Schema is the element's attribute tree, everything computed. Schema *AttributeTree `json:"schema"` + // AddressingSchema is the addressing attributes the collection path + // requires, declared as the list block's own configuration. Nil for a + // collection path that takes no parameters. + AddressingSchema *AttributeTree `json:"addressing_schema,omitempty"` // CoManagementNote is the sibling-entity prose; see Resource. CoManagementNote string `json:"co_management_note,omitempty"` // ListEnvelopeKey is the list response's item-array wrapper key; see diff --git a/internal/templates/services/list-resource/list.go.tmpl b/internal/templates/services/list-resource/list.go.tmpl index 205e52c..3417237 100644 --- a/internal/templates/services/list-resource/list.go.tmpl +++ b/internal/templates/services/list-resource/list.go.tmpl @@ -8,6 +8,14 @@ package {{ .Package }} // element, carrying its identity and display name. func (r *{{ .Type }}) List(ctx context.Context, req list.ListRequest, stream *list.ListResultsStream) { client := r.client +{{- if .ConfigModel }} + var config listConfigModel + if diags := req.Config.Get(ctx, &config); diags.HasError() { + stream.Results = list.ListResultsStreamDiagnostics(diags) + return + } + {{ .ListPlan.ParamDecls }} +{{- end }} {{ .ListPlan.Assign }} if err != nil { stream.Results = list.ListResultsStreamDiagnostics(diag.Diagnostics{ diff --git a/internal/templates/services/list-resource/list_resource.go.tmpl b/internal/templates/services/list-resource/list_resource.go.tmpl index 3b8b569..4cc4235 100644 --- a/internal/templates/services/list-resource/list_resource.go.tmpl +++ b/internal/templates/services/list-resource/list_resource.go.tmpl @@ -43,9 +43,14 @@ func (r *{{ .Type }}) Configure(_ context.Context, req resource.ConfigureRequest } // ListResourceConfigSchema declares the list block's configuration: the -// document derives no filters for this entity, so the block is empty. +// addressing the collection path requires, and nothing else. An entity whose +// collection path takes no parameters declares an empty block. func (r *{{ .Type }}) ListResourceConfigSchema(_ context.Context, _ list.ListResourceSchemaRequest, resp *list.ListResourceSchemaResponse) { resp.Schema = listschema.Schema{ MarkdownDescription: {{ .SchemaDescription }}, +{{- if .SchemaAttributes }} + Attributes: map[string]listschema.Attribute{ +{{ .SchemaAttributes }} }, +{{- end }} } } diff --git a/internal/templates/services/list-resource/list_resource_test.go.tmpl b/internal/templates/services/list-resource/list_resource_test.go.tmpl index a9a7d62..23b4dd0 100644 --- a/internal/templates/services/list-resource/list_resource_test.go.tmpl +++ b/internal/templates/services/list-resource/list_resource_test.go.tmpl @@ -33,6 +33,20 @@ func TestUnit{{ .Pascal }}ListResource_Schema(t *testing.T) { t.Fatalf("schema diagnostics: %v", schemaResp.Diagnostics) } } +{{ if .ConfigValue }} +// listConfig is the addressing the collection path requires, at the derived +// fixture values, as terraform would supply it. +func listConfig(t *testing.T, lr list.ListResource) tfsdk.Config { + t.Helper() + schemaResp := &list.ListResourceSchemaResponse{} + lr.ListResourceConfigSchema(context.Background(), list.ListResourceSchemaRequest{}, schemaResp) + if schemaResp.Diagnostics.HasError() { + t.Fatalf("schema diagnostics: %v", schemaResp.Diagnostics) + } + raw := {{ .ConfigValue }} + return tfsdk.Config{Schema: schemaResp.Schema, Raw: raw} +} +{{- end }} // TestUnit{{ .Pascal }}ListResource_List streams the mock collection and // holds the results to the derived fixture values. @@ -47,9 +61,15 @@ func TestUnit{{ .Pascal }}ListResource_List(t *testing.T) { // the content type, and httpmock's string responder announces text/plain, // which no SDK has a parser for. The body is embedded rather than // marshalled, so the JSON responder — which takes a value — does not fit. +{{- if .CollectionPattern }} + httpmock.RegisterResponder("GET", `{{ .CollectionPattern }}`, + httpmock.NewStringResponder(200, listResponse). + HeaderSet(http.Header{"Content-Type": []string{"application/json"}})) +{{- else }} httpmock.RegisterResponder("GET", "{{ .CollectionURL }}", httpmock.NewStringResponder(200, listResponse). HeaderSet(http.Header{"Content-Type": []string{"application/json"}})) +{{- end }} sdkClient, err := client.NewSDKClient(client.Config{Endpoint: mocks.UnitEndpoint, UserAgent: "unit-test"{{ .TestClientConfig }}}) if err != nil { @@ -68,7 +88,12 @@ func TestUnit{{ .Pascal }}ListResource_List(t *testing.T) { } stream := &list.ListResultsStream{} - lr.List(ctx, list.ListRequest{ResourceIdentitySchema: identitySchema}, stream) + lr.List(ctx, list.ListRequest{ + ResourceIdentitySchema: identitySchema, +{{- if .ConfigValue }} + Config: listConfig(t, lr), +{{- end }} + }, stream) var ids []string for result := range stream.Results { diff --git a/internal/templates/services/list-resource/model.go.tmpl b/internal/templates/services/list-resource/model.go.tmpl index 208c2d5..b021534 100644 --- a/internal/templates/services/list-resource/model.go.tmpl +++ b/internal/templates/services/list-resource/model.go.tmpl @@ -10,3 +10,7 @@ import ( type identityModel struct { ID types.String `tfsdk:"id"` } +{{ if .ConfigModel }} +// listConfigModel mirrors the list block's configuration. +{{ .ConfigModel }} +{{- end }}