diff --git a/DESIGN.md b/DESIGN.md index ca44e24..f25c6d3 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -51,6 +51,8 @@ In these OAS operations, the generator will search the `create` and `read` for s 4. `read` operation: [parameters](https://spec.openapis.org/oas/v3.1.0#parameterObject) - The generator will merge all `query` and `path` parameters to the root of the schema. - The generator will consider as parameters the ones in the [OAS Path Item](https://spec.openapis.org/oas/v3.1.0#path-item-object) and the ones in the [OAS Operation](https://spec.openapis.org/oas/v3.1.0#operation-object), merged based on the rules in the specification + - Two parameters that map to the same attribute name, like a `path` and a `query` parameter both named `zone`, or a parameter aliased onto another's name, cannot both be kept, and neither can win without silently discarding the other. The resource is skipped with a warning, and the specification or the aliases must be fixed. + - The `create` operation's `path` parameters are read to decide whether a `read` operation `path` parameter is a practitioner input, as described in [Resources - Required, Computed or Optional](#resources---required-computed-or-optional). They are not themselves mapped to attributes. All schemas found will be deep merged together, with the `requestBody` schema from the `create` operation being the **main schema** that the others will be merged on top. The deep merge has the following characteristics: @@ -74,6 +76,7 @@ The generator uses the `read` operation to map to the provider code specificatio 1. `read` operation: [parameters](https://spec.openapis.org/oas/v3.1.0#parameterObject) - The generator will merge all `query` and `path` parameters to the root of the schema. - The generator will consider as parameters the ones in the [Path Item Object](https://spec.openapis.org/oas/v3.1.0#path-item-object) and the ones in the [Operation Object](https://spec.openapis.org/oas/v3.1.0#operation-object), merged based on the rules in the specification + - Two parameters that map to the same attribute name, like a `path` and a `query` parameter both named `zone`, or a parameter aliased onto another's name, cannot both be kept, and neither can win without silently discarding the other. The data source is skipped with a warning, and the specification or the aliases must be fixed. 2. `read` operation: response body in [responses](https://spec.openapis.org/oas/v3.1.0#responsesObject) - The response body is the only schema **required** for data sources. If not found, the generator will skip the data source without mapping. - Will attempt to use `200` or `201` response body. If not found, will grab the first available `2xx` response code with a schema (lexicographic order) @@ -215,6 +218,12 @@ If not required, then the field will be mapped as `computed_optional`. If the field is only present in a schema other than the `create` operation `requestBody`, then the field will be mapped as `computed`. +A `read` operation `path` parameter marked as `required` that also appears as a `path` parameter of the `create` operation will be mapped as `required`. Such a parameter is addressed by the URL the `create` operation is sent to, so the practitioner has to supply it. Any other `read` operation parameter will be mapped as `computed_optional`, including a `required` `path` parameter that only appears on the `read` path -- that one identifies something the API assigned. + +`required` on a parameter is mandatory for `path` parameters, so it cannot distinguish the two cases on its own. On `/parents/{parent_id}/children/{child_id}`, created with `POST /parents/{parent_id}/children`, `parent_id` is `required` and `child_id` is `computed_optional`. + +A `required` mapped from a parameter also takes precedence over any other classification of the same field name, even though the parameters are otherwise the lowest precedence schema. That covers a `computed` mapped from a response body as well as a `computed_optional` mapped from a create request body property that is not `required`: a value addressed by the create URL has to be supplied either way, so discarding the parameter's `required` would leave an attribute the API needs as an input mapped as something the practitioner may omit. + #### Data Sources - Required, Computed or Optional For data sources, all fields in the `read` operation `parameters` OAS schema marked as [required](https://json-schema.org/understanding-json-schema/reference/object.html#required-properties) will be mapped as `required`. diff --git a/internal/cmd/testdata/edgecase/generator_config.yml b/internal/cmd/testdata/edgecase/generator_config.yml index 02fc1b9..330118e 100644 --- a/internal/cmd/testdata/edgecase/generator_config.yml +++ b/internal/cmd/testdata/edgecase/generator_config.yml @@ -33,6 +33,14 @@ resources: path: /composition_siblings method: GET + nested_path: + create: + path: /parents/{parent_id}/children + method: POST + read: + path: /parents/{parent_id}/children/{child_id} + method: GET + data_sources: nested_collections: read: diff --git a/internal/cmd/testdata/edgecase/openapi_spec.yml b/internal/cmd/testdata/edgecase/openapi_spec.yml index 9100d65..ff967e1 100644 --- a/internal/cmd/testdata/edgecase/openapi_spec.yml +++ b/internal/cmd/testdata/edgecase/openapi_spec.yml @@ -199,8 +199,75 @@ paths: deprecated: true allOf: - $ref: "#/components/schemas/composition_refinable_object" + /parents/{parent_id}/children: + # Declared on the path item rather than the operation, so that resolving the create + # operation's path parameters has to merge the path item in. + parameters: + - name: parent_id + in: path + description: The parent this child hangs off, supplied by the practitioner + required: true + schema: + type: string + post: + summary: Create a child under a parent addressed only by the URL + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - name + properties: + name: + description: The child's name + type: string + responses: + "201": + description: Created + content: + application/json: + schema: + $ref: "#/components/schemas/child_object" + /parents/{parent_id}/children/{child_id}: + get: + summary: Both path parameters are required, but only parent_id is a practitioner input + parameters: + - name: parent_id + in: path + description: The parent this child hangs off, supplied by the practitioner + required: true + schema: + type: string + - name: child_id + in: path + description: The child's identifier, assigned by the API + required: true + schema: + type: string + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: "#/components/schemas/child_object" components: schemas: + child_object: + description: Echoes both path parameters back, so the response reaches the attributes first + type: object + properties: + parent_id: + description: The response is merged first, so this description outlives the parameter's + type: string + child_id: + description: The response is merged first, so this description outlives the parameter's + type: string + name: + description: The child's name + type: string composition_object: description: This description loses to the composing schema's type: object diff --git a/internal/cmd/testdata/edgecase/provider_code_spec.json b/internal/cmd/testdata/edgecase/provider_code_spec.json index a22eb7a..6c4fdcb 100644 --- a/internal/cmd/testdata/edgecase/provider_code_spec.json +++ b/internal/cmd/testdata/edgecase/provider_code_spec.json @@ -532,6 +532,34 @@ ] } }, + { + "name": "nested_path", + "schema": { + "attributes": [ + { + "name": "name", + "string": { + "computed_optional_required": "required", + "description": "The child's name" + } + }, + { + "name": "child_id", + "string": { + "computed_optional_required": "computed", + "description": "The response is merged first, so this description outlives the parameter's" + } + }, + { + "name": "parent_id", + "string": { + "computed_optional_required": "required", + "description": "The response is merged first, so this description outlives the parameter's" + } + } + ] + } + }, { "name": "set_test", "schema": { diff --git a/internal/cmd/testdata/kubernetes/provider_code_spec.json b/internal/cmd/testdata/kubernetes/provider_code_spec.json index 1183a6e..5973140 100644 --- a/internal/cmd/testdata/kubernetes/provider_code_spec.json +++ b/internal/cmd/testdata/kubernetes/provider_code_spec.json @@ -9253,7 +9253,7 @@ { "name": "namespace", "string": { - "computed_optional_required": "computed_optional", + "computed_optional_required": "required", "description": "object name and auth scope, such as for teams and projects" } }, diff --git a/internal/cmd/testdata/scaleway/provider_code_spec.json b/internal/cmd/testdata/scaleway/provider_code_spec.json index b2c76ec..5d98056 100644 --- a/internal/cmd/testdata/scaleway/provider_code_spec.json +++ b/internal/cmd/testdata/scaleway/provider_code_spec.json @@ -161,7 +161,7 @@ { "name": "zone", "string": { - "computed_optional_required": "computed_optional", + "computed_optional_required": "required", "description": "The zone you want to target", "validators": [ { @@ -302,7 +302,7 @@ { "name": "zone", "string": { - "computed_optional_required": "computed_optional", + "computed_optional_required": "required", "description": "The zone you want to target", "validators": [ { diff --git a/internal/explorer/config_explorer.go b/internal/explorer/config_explorer.go index 27a1862..ea2d32f 100644 --- a/internal/explorer/config_explorer.go +++ b/internal/explorer/config_explorer.go @@ -87,14 +87,21 @@ func (e configExplorer) FindResources() (map[string]Resource, error) { continue } + createCommonParameters, err := extractCommonParameters(e.spec.Paths, resourceConfig.Create.Path) + if err != nil { + errResult = errors.Join(errResult, fmt.Errorf("failed to extract '%s' create common parameters: %w", name, err)) + continue + } + resources[name] = Resource{ - CreateOp: createOp, - ReadOp: readOp, - UpdateOp: updateOp, - DeleteOp: deleteOp, - CommonParameters: commonParameters, - SchemaOptions: extractSchemaOptions(resourceConfig.SchemaOptions), - Description: getOperationTagDescription(e.spec, createOp), + CreateOp: createOp, + ReadOp: readOp, + UpdateOp: updateOp, + DeleteOp: deleteOp, + CommonParameters: commonParameters, + CreateCommonParameters: createCommonParameters, + SchemaOptions: extractSchemaOptions(resourceConfig.SchemaOptions), + Description: getOperationTagDescription(e.spec, createOp), } } diff --git a/internal/explorer/config_explorer_test.go b/internal/explorer/config_explorer_test.go index 1517bb5..33064d5 100644 --- a/internal/explorer/config_explorer_test.go +++ b/internal/explorer/config_explorer_test.go @@ -174,6 +174,67 @@ func Test_ConfigExplorer_FindResources(t *testing.T) { }, }, }, + "path item parameters are extracted per operation path": { + config: config.Config{ + Resources: map[string]config.Resource{ + "test_resource": { + Create: &config.OpenApiSpecLocation{ + Path: "/parents/{parent_id}/resources", + Method: "POST", + }, + Read: &config.OpenApiSpecLocation{ + Path: "/parents/{parent_id}/resources/{resource_id}", + Method: "GET", + }, + }, + }, + }, + pathItems: orderedmap.ToOrderedMap(map[string]*high.PathItem{ + "/parents/{parent_id}/resources": { + Parameters: []*high.Parameter{ + {Name: "parent_id", In: "path", Required: new(true)}, + }, + Post: &high.Operation{ + Description: "create op here", + OperationId: "create_resource", + }, + }, + "/parents/{parent_id}/resources/{resource_id}": { + Parameters: []*high.Parameter{ + {Name: "parent_id", In: "path", Required: new(true)}, + {Name: "resource_id", In: "path", Required: new(true)}, + }, + Get: &high.Operation{ + Description: "read op here", + OperationId: "read_resource", + }, + }, + }), + want: map[string]explorer.Resource{ + "test_resource": { + CreateOp: &high.Operation{ + Description: "create op here", + OperationId: "create_resource", + }, + ReadOp: &high.Operation{ + Description: "read op here", + OperationId: "read_resource", + }, + CommonParameters: []*high.Parameter{ + {Name: "parent_id", In: "path", Required: new(true)}, + {Name: "resource_id", In: "path", Required: new(true)}, + }, + CreateCommonParameters: []*high.Parameter{ + {Name: "parent_id", In: "path", Required: new(true)}, + }, + SchemaOptions: explorer.SchemaOptions{ + AttributeOptions: explorer.AttributeOptions{ + Overrides: map[string]explorer.Override{}, + }, + }, + }, + }, + }, "non-existent create path throws error": { config: config.Config{ Resources: map[string]config.Resource{ @@ -341,7 +402,7 @@ func Test_ConfigExplorer_FindResources(t *testing.T) { return } - if diff := cmp.Diff(got, testCase.want, cmpopts.IgnoreUnexported(high.Operation{})); testCase.expectedErr == nil && diff != "" { + if diff := cmp.Diff(got, testCase.want, cmpopts.IgnoreUnexported(high.Operation{}, high.Parameter{})); testCase.expectedErr == nil && diff != "" { t.Errorf("unexpected difference: %s", diff) } }) diff --git a/internal/explorer/explorer.go b/internal/explorer/explorer.go index 2e4fc8e..15920d2 100644 --- a/internal/explorer/explorer.go +++ b/internal/explorer/explorer.go @@ -17,13 +17,16 @@ type Explorer interface { // Resource contains CRUD operations and schema options for configuration. type Resource struct { - CreateOp *high.Operation - ReadOp *high.Operation - UpdateOp *high.Operation - DeleteOp *high.Operation + CreateOp *high.Operation + ReadOp *high.Operation + UpdateOp *high.Operation + DeleteOp *high.Operation + // CommonParameters are the path item parameters of the read path. CommonParameters []*high.Parameter - SchemaOptions SchemaOptions - Description string // Resource-level description from OpenAPI tags + // CreateCommonParameters are the path item parameters of the create path. + CreateCommonParameters []*high.Parameter + SchemaOptions SchemaOptions + Description string // Resource-level description from OpenAPI tags } // DataSource contains a Read operation and schema options for configuration. diff --git a/internal/explorer/explorer_utils.go b/internal/explorer/explorer_utils.go index 51e1257..7e5ea55 100644 --- a/internal/explorer/explorer_utils.go +++ b/internal/explorer/explorer_utils.go @@ -12,7 +12,11 @@ func mergeParameters(commonParameters []*high.Parameter, operation *high.Operati for _, operationParameter := range operation.Parameters { found := false for i, mergedParameter := range mergedParameters { - if operationParameter.Name == mergedParameter.Name { + // A parameter is identified by name and location, so an operation parameter only + // overrides a path item parameter when both match. Matching on the name alone + // would let, say, an operation's `zone` query parameter displace the path item's + // `zone` path parameter, which the specification treats as a different parameter. + if operationParameter.Name == mergedParameter.Name && operationParameter.In == mergedParameter.In { found = true mergedParameters[i] = operationParameter break @@ -30,6 +34,10 @@ func (e *Resource) ReadOpParameters() []*high.Parameter { return mergeParameters(e.CommonParameters, e.ReadOp) } +func (e *Resource) CreateOpParameters() []*high.Parameter { + return mergeParameters(e.CreateCommonParameters, e.CreateOp) +} + func (e *DataSource) ReadOpParameters() []*high.Parameter { return mergeParameters(e.CommonParameters, e.ReadOp) } diff --git a/internal/explorer/explorer_utils_test.go b/internal/explorer/explorer_utils_test.go index 2a743f5..6389d64 100644 --- a/internal/explorer/explorer_utils_test.go +++ b/internal/explorer/explorer_utils_test.go @@ -23,6 +23,48 @@ func TestReadOpParameters_Resource(t *testing.T) { commonParams []*high.Parameter want []*high.Parameter }{ + "same name in different locations - both kept": { + readOp: &high.Operation{ + Parameters: []*high.Parameter{ + { + Name: "zone", + Required: new(false), + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + }, + commonParams: []*high.Parameter{ + { + Name: "zone", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: []*high.Parameter{ + { + Name: "zone", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + Name: "zone", + Required: new(false), + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + }, "merge common and operation": { readOp: &high.Operation{ Parameters: []*high.Parameter{ @@ -343,6 +385,171 @@ func TestReadOpParameters_Resource(t *testing.T) { } } +func TestCreateOpParameters_Resource(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + createOp *high.Operation + commonParams []*high.Parameter + want []*high.Parameter + }{ + "same name in different locations - both kept": { + createOp: &high.Operation{ + Parameters: []*high.Parameter{ + { + Name: "zone", + Required: new(false), + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + }, + commonParams: []*high.Parameter{ + { + Name: "zone", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: []*high.Parameter{ + { + Name: "zone", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + Name: "zone", + Required: new(false), + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + }, + "merge common and operation": { + createOp: &high.Operation{ + Parameters: []*high.Parameter{ + { + Name: "operation_prop", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + Name: "shared_prop", + Required: new(true), + In: "path", + Description: "the operation wins over the path item", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + }, + commonParams: []*high.Parameter{ + { + Name: "common_prop", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + Name: "shared_prop", + Required: new(true), + In: "path", + Description: "you shouldn't see this, the operation overrides it", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: []*high.Parameter{ + { + Name: "common_prop", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + Name: "shared_prop", + Required: new(true), + In: "path", + Description: "the operation wins over the path item", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + Name: "operation_prop", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + }, + "path item only": { + createOp: &high.Operation{}, + commonParams: []*high.Parameter{ + { + Name: "common_prop", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: []*high.Parameter{ + { + Name: "common_prop", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + }, + "no create operation": { + createOp: nil, + want: []*high.Parameter{}, + }, + } + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + resource := Resource{ + CreateOp: testCase.createOp, + CreateCommonParameters: testCase.commonParams, + } + + mergedParameters := resource.CreateOpParameters() + + if diff := cmp.Diff(mergedParameters, testCase.want, cmpopts.IgnoreUnexported(sync.Mutex{}), cmp.AllowUnexported(base.Schema{}, base.SchemaProxy{}, high.Parameter{})); diff != "" { + t.Errorf("unexpected difference for resource: %s", diff) + } + }) + } +} + func TestReadOpParameters_DataSource(t *testing.T) { t.Parallel() @@ -351,6 +558,48 @@ func TestReadOpParameters_DataSource(t *testing.T) { commonParams []*high.Parameter want []*high.Parameter }{ + "same name in different locations - both kept": { + readOp: &high.Operation{ + Parameters: []*high.Parameter{ + { + Name: "zone", + Required: new(false), + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + }, + commonParams: []*high.Parameter{ + { + Name: "zone", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: []*high.Parameter{ + { + Name: "zone", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + Name: "zone", + Required: new(false), + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + }, "merge common and operation": { readOp: &high.Operation{ Parameters: []*high.Parameter{ diff --git a/internal/mapper/attrmapper/bool.go b/internal/mapper/attrmapper/bool.go index 5e3d01e..65d466e 100644 --- a/internal/mapper/attrmapper/bool.go +++ b/internal/mapper/attrmapper/bool.go @@ -24,9 +24,14 @@ func (a *ResourceBoolAttribute) GetName() string { func (a *ResourceBoolAttribute) Merge(mergeAttribute ResourceAttribute) (ResourceAttribute, error) { boolAttribute, ok := mergeAttribute.(*ResourceBoolAttribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = boolAttribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, boolAttribute.ComputedOptionalRequired) return a, nil } @@ -57,9 +62,14 @@ func (a *DataSourceBoolAttribute) GetName() string { func (a *DataSourceBoolAttribute) Merge(mergeAttribute DataSourceAttribute) (DataSourceAttribute, error) { boolAttribute, ok := mergeAttribute.(*DataSourceBoolAttribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = boolAttribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, boolAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/bool_test.go b/internal/mapper/attrmapper/bool_test.go index a2d491c..53ee4fa 100644 --- a/internal/mapper/attrmapper/bool_test.go +++ b/internal/mapper/attrmapper/bool_test.go @@ -67,6 +67,29 @@ func TestResourceBoolAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceBoolAttribute{ + Name: "bool_attribute", + BoolAttribute: resource.BoolAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old bool description"), + }, + }, + mergeAttribute: &attrmapper.ResourceBoolAttribute{ + Name: "bool_attribute", + BoolAttribute: resource.BoolAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new bool description"), + }, + }, + expectedAttribute: &attrmapper.ResourceBoolAttribute{ + Name: "bool_attribute", + BoolAttribute: resource.BoolAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old bool description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceBoolAttribute{ Name: "bool_attribute", @@ -221,6 +244,29 @@ func TestDataSourceBoolAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceBoolAttribute{ + Name: "bool_attribute", + BoolAttribute: datasource.BoolAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old bool description"), + }, + }, + mergeAttribute: &attrmapper.DataSourceBoolAttribute{ + Name: "bool_attribute", + BoolAttribute: datasource.BoolAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new bool description"), + }, + }, + expectedAttribute: &attrmapper.DataSourceBoolAttribute{ + Name: "bool_attribute", + BoolAttribute: datasource.BoolAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old bool description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceBoolAttribute{ Name: "bool_attribute", diff --git a/internal/mapper/attrmapper/data_source_attributes_test.go b/internal/mapper/attrmapper/data_source_attributes_test.go index 520411e..2cebd05 100644 --- a/internal/mapper/attrmapper/data_source_attributes_test.go +++ b/internal/mapper/attrmapper/data_source_attributes_test.go @@ -86,6 +86,79 @@ func TestDataSourceAttributes_Merge(t *testing.T) { }, }, }, + "required in a later merge slice - promote computability": { + targetAttributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "string_attribute", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("string description"), + }, + }, + &attrmapper.DataSourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceBoolAttribute{ + Name: "nested_bool", + BoolAttribute: datasource.BoolAttribute{ + ComputedOptionalRequired: schema.Computed, + }, + }, + }, + SingleNestedAttribute: datasource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + }, + }, + }, + mergeAttributeSlices: []attrmapper.DataSourceAttributes{ + { + &attrmapper.DataSourceStringAttribute{ + Name: "string_attribute", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("this will be ignored"), + }, + }, + &attrmapper.DataSourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceBoolAttribute{ + Name: "nested_bool", + BoolAttribute: datasource.BoolAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + SingleNestedAttribute: datasource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + }, + expectedAttributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "string_attribute", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("string description"), + }, + }, + &attrmapper.DataSourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceBoolAttribute{ + Name: "nested_bool", + BoolAttribute: datasource.BoolAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + SingleNestedAttribute: datasource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + }, "recursive - matches and appends": { targetAttributes: attrmapper.DataSourceAttributes{ &attrmapper.DataSourceSingleNestedAttribute{ diff --git a/internal/mapper/attrmapper/float64.go b/internal/mapper/attrmapper/float64.go index f457fef..4130b42 100644 --- a/internal/mapper/attrmapper/float64.go +++ b/internal/mapper/attrmapper/float64.go @@ -24,9 +24,14 @@ func (a *ResourceFloat64Attribute) GetName() string { func (a *ResourceFloat64Attribute) Merge(mergeAttribute ResourceAttribute) (ResourceAttribute, error) { float64Attribute, ok := mergeAttribute.(*ResourceFloat64Attribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = float64Attribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, float64Attribute.ComputedOptionalRequired) return a, nil } @@ -57,9 +62,14 @@ func (a *DataSourceFloat64Attribute) GetName() string { func (a *DataSourceFloat64Attribute) Merge(mergeAttribute DataSourceAttribute) (DataSourceAttribute, error) { float64Attribute, ok := mergeAttribute.(*DataSourceFloat64Attribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = float64Attribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, float64Attribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/float64_test.go b/internal/mapper/attrmapper/float64_test.go index 14063b3..10a3832 100644 --- a/internal/mapper/attrmapper/float64_test.go +++ b/internal/mapper/attrmapper/float64_test.go @@ -67,6 +67,29 @@ func TestResourceFloat64Attribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceFloat64Attribute{ + Name: "float64_attribute", + Float64Attribute: resource.Float64Attribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old float64 description"), + }, + }, + mergeAttribute: &attrmapper.ResourceFloat64Attribute{ + Name: "float64_attribute", + Float64Attribute: resource.Float64Attribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new float64 description"), + }, + }, + expectedAttribute: &attrmapper.ResourceFloat64Attribute{ + Name: "float64_attribute", + Float64Attribute: resource.Float64Attribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old float64 description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceFloat64Attribute{ Name: "float64_attribute", @@ -221,6 +244,29 @@ func TestDataSourceFloat64Attribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceFloat64Attribute{ + Name: "float64_attribute", + Float64Attribute: datasource.Float64Attribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old float64 description"), + }, + }, + mergeAttribute: &attrmapper.DataSourceFloat64Attribute{ + Name: "float64_attribute", + Float64Attribute: datasource.Float64Attribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new float64 description"), + }, + }, + expectedAttribute: &attrmapper.DataSourceFloat64Attribute{ + Name: "float64_attribute", + Float64Attribute: datasource.Float64Attribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old float64 description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceFloat64Attribute{ Name: "float64_attribute", diff --git a/internal/mapper/attrmapper/int64.go b/internal/mapper/attrmapper/int64.go index 05b73a9..867c9e1 100644 --- a/internal/mapper/attrmapper/int64.go +++ b/internal/mapper/attrmapper/int64.go @@ -24,9 +24,14 @@ func (a *ResourceInt64Attribute) GetName() string { func (a *ResourceInt64Attribute) Merge(mergeAttribute ResourceAttribute) (ResourceAttribute, error) { int64Attribute, ok := mergeAttribute.(*ResourceInt64Attribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = int64Attribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, int64Attribute.ComputedOptionalRequired) return a, nil } @@ -57,9 +62,14 @@ func (a *DataSourceInt64Attribute) GetName() string { func (a *DataSourceInt64Attribute) Merge(mergeAttribute DataSourceAttribute) (DataSourceAttribute, error) { int64Attribute, ok := mergeAttribute.(*DataSourceInt64Attribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = int64Attribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, int64Attribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/int64_test.go b/internal/mapper/attrmapper/int64_test.go index 8514316..1fd3697 100644 --- a/internal/mapper/attrmapper/int64_test.go +++ b/internal/mapper/attrmapper/int64_test.go @@ -67,6 +67,29 @@ func TestResourceInt64Attribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceInt64Attribute{ + Name: "int64_attribute", + Int64Attribute: resource.Int64Attribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old int64 description"), + }, + }, + mergeAttribute: &attrmapper.ResourceInt64Attribute{ + Name: "int64_attribute", + Int64Attribute: resource.Int64Attribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new int64 description"), + }, + }, + expectedAttribute: &attrmapper.ResourceInt64Attribute{ + Name: "int64_attribute", + Int64Attribute: resource.Int64Attribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old int64 description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceInt64Attribute{ Name: "int64_attribute", @@ -221,6 +244,29 @@ func TestDataSourceInt64Attribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceInt64Attribute{ + Name: "int64_attribute", + Int64Attribute: datasource.Int64Attribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old int64 description"), + }, + }, + mergeAttribute: &attrmapper.DataSourceInt64Attribute{ + Name: "int64_attribute", + Int64Attribute: datasource.Int64Attribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new int64 description"), + }, + }, + expectedAttribute: &attrmapper.DataSourceInt64Attribute{ + Name: "int64_attribute", + Int64Attribute: datasource.Int64Attribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old int64 description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceInt64Attribute{ Name: "int64_attribute", diff --git a/internal/mapper/attrmapper/list.go b/internal/mapper/attrmapper/list.go index 1d717f2..4843487 100644 --- a/internal/mapper/attrmapper/list.go +++ b/internal/mapper/attrmapper/list.go @@ -32,6 +32,7 @@ func (a *ResourceListAttribute) Merge(mergeAttribute ResourceAttribute) (Resourc a.Description = listAttribute.Description } a.ElementType = mergeElementType(a.ElementType, listAttribute.ElementType) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, listAttribute.ComputedOptionalRequired) return a, nil } @@ -70,6 +71,7 @@ func (a *DataSourceListAttribute) Merge(mergeAttribute DataSourceAttribute) (Dat a.Description = listAttribute.Description } a.ElementType = mergeElementType(a.ElementType, listAttribute.ElementType) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, listAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/list_nested.go b/internal/mapper/attrmapper/list_nested.go index 54ea7bf..24d797a 100644 --- a/internal/mapper/attrmapper/list_nested.go +++ b/internal/mapper/attrmapper/list_nested.go @@ -33,6 +33,7 @@ func (a *ResourceListNestedAttribute) Merge(mergeAttribute ResourceAttribute) (R a.Description = listNestedAttribute.Description } a.NestedObject.Attributes, _ = a.NestedObject.Attributes.Merge(listNestedAttribute.NestedObject.Attributes) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, listNestedAttribute.ComputedOptionalRequired) return a, nil } @@ -83,6 +84,7 @@ func (a *DataSourceListNestedAttribute) Merge(mergeAttribute DataSourceAttribute a.Description = listNestedAttribute.Description } a.NestedObject.Attributes, _ = a.NestedObject.Attributes.Merge(listNestedAttribute.NestedObject.Attributes) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, listNestedAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/list_nested_test.go b/internal/mapper/attrmapper/list_nested_test.go index f48fa6b..02b5bc2 100644 --- a/internal/mapper/attrmapper/list_nested_test.go +++ b/internal/mapper/attrmapper/list_nested_test.go @@ -131,6 +131,62 @@ func TestResourceListNestedAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceListNestedAttribute{ + Name: "list_nested_attribute", + NestedObject: attrmapper.ResourceNestedAttributeObject{ + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old nested string description"), + }, + }, + }, + }, + ListNestedAttribute: resource.ListNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old list nested description"), + }, + }, + mergeAttribute: &attrmapper.ResourceListNestedAttribute{ + Name: "list_nested_attribute", + NestedObject: attrmapper.ResourceNestedAttributeObject{ + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new nested string description"), + }, + }, + }, + }, + ListNestedAttribute: resource.ListNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new list nested description"), + }, + }, + expectedAttribute: &attrmapper.ResourceListNestedAttribute{ + Name: "list_nested_attribute", + NestedObject: attrmapper.ResourceNestedAttributeObject{ + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old nested string description"), + }, + }, + }, + }, + ListNestedAttribute: resource.ListNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old list nested description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceListNestedAttribute{ Name: "list_nested_attribute", @@ -689,6 +745,62 @@ func TestDataSourceListNestedAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceListNestedAttribute{ + Name: "list_nested_attribute", + NestedObject: attrmapper.DataSourceNestedAttributeObject{ + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old nested string description"), + }, + }, + }, + }, + ListNestedAttribute: datasource.ListNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old list nested description"), + }, + }, + mergeAttribute: &attrmapper.DataSourceListNestedAttribute{ + Name: "list_nested_attribute", + NestedObject: attrmapper.DataSourceNestedAttributeObject{ + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new nested string description"), + }, + }, + }, + }, + ListNestedAttribute: datasource.ListNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new list nested description"), + }, + }, + expectedAttribute: &attrmapper.DataSourceListNestedAttribute{ + Name: "list_nested_attribute", + NestedObject: attrmapper.DataSourceNestedAttributeObject{ + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old nested string description"), + }, + }, + }, + }, + ListNestedAttribute: datasource.ListNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old list nested description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceListNestedAttribute{ Name: "list_nested_attribute", diff --git a/internal/mapper/attrmapper/list_test.go b/internal/mapper/attrmapper/list_test.go index 5a6b997..975e9cf 100644 --- a/internal/mapper/attrmapper/list_test.go +++ b/internal/mapper/attrmapper/list_test.go @@ -114,6 +114,38 @@ func TestResourceListAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceListAttribute{ + Name: "list_attribute", + ListAttribute: resource.ListAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old list description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + mergeAttribute: &attrmapper.ResourceListAttribute{ + Name: "list_attribute", + ListAttribute: resource.ListAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new list description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + expectedAttribute: &attrmapper.ResourceListAttribute{ + Name: "list_attribute", + ListAttribute: resource.ListAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old list description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceListAttribute{ Name: "list_attribute", @@ -628,6 +660,38 @@ func TestDataSourceListAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceListAttribute{ + Name: "list_attribute", + ListAttribute: datasource.ListAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old list description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + mergeAttribute: &attrmapper.DataSourceListAttribute{ + Name: "list_attribute", + ListAttribute: datasource.ListAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new list description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + expectedAttribute: &attrmapper.DataSourceListAttribute{ + Name: "list_attribute", + ListAttribute: datasource.ListAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old list description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceListAttribute{ Name: "list_attribute", diff --git a/internal/mapper/attrmapper/map.go b/internal/mapper/attrmapper/map.go index e0a1051..39f5d45 100644 --- a/internal/mapper/attrmapper/map.go +++ b/internal/mapper/attrmapper/map.go @@ -32,6 +32,7 @@ func (a *ResourceMapAttribute) Merge(mergeAttribute ResourceAttribute) (Resource a.Description = mapAttribute.Description } a.ElementType = mergeElementType(a.ElementType, mapAttribute.ElementType) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, mapAttribute.ComputedOptionalRequired) return a, nil } @@ -70,6 +71,7 @@ func (a *DataSourceMapAttribute) Merge(mergeAttribute DataSourceAttribute) (Data a.Description = mapAttribute.Description } a.ElementType = mergeElementType(a.ElementType, mapAttribute.ElementType) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, mapAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/map_nested.go b/internal/mapper/attrmapper/map_nested.go index 600069d..0a8fd7b 100644 --- a/internal/mapper/attrmapper/map_nested.go +++ b/internal/mapper/attrmapper/map_nested.go @@ -33,6 +33,7 @@ func (a *ResourceMapNestedAttribute) Merge(mergeAttribute ResourceAttribute) (Re a.Description = mapNestedAttribute.Description } a.NestedObject.Attributes, _ = a.NestedObject.Attributes.Merge(mapNestedAttribute.NestedObject.Attributes) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, mapNestedAttribute.ComputedOptionalRequired) return a, nil } @@ -83,6 +84,7 @@ func (a *DataSourceMapNestedAttribute) Merge(mergeAttribute DataSourceAttribute) a.Description = mapNestedAttribute.Description } a.NestedObject.Attributes, _ = a.NestedObject.Attributes.Merge(mapNestedAttribute.NestedObject.Attributes) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, mapNestedAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/map_nested_test.go b/internal/mapper/attrmapper/map_nested_test.go index ecc29ca..508698b 100644 --- a/internal/mapper/attrmapper/map_nested_test.go +++ b/internal/mapper/attrmapper/map_nested_test.go @@ -131,6 +131,62 @@ func TestResourceMapNestedAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceMapNestedAttribute{ + Name: "map_nested_attribute", + NestedObject: attrmapper.ResourceNestedAttributeObject{ + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old nested string description"), + }, + }, + }, + }, + MapNestedAttribute: resource.MapNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old map nested description"), + }, + }, + mergeAttribute: &attrmapper.ResourceMapNestedAttribute{ + Name: "map_nested_attribute", + NestedObject: attrmapper.ResourceNestedAttributeObject{ + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new nested string description"), + }, + }, + }, + }, + MapNestedAttribute: resource.MapNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new map nested description"), + }, + }, + expectedAttribute: &attrmapper.ResourceMapNestedAttribute{ + Name: "map_nested_attribute", + NestedObject: attrmapper.ResourceNestedAttributeObject{ + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old nested string description"), + }, + }, + }, + }, + MapNestedAttribute: resource.MapNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old map nested description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceMapNestedAttribute{ Name: "map_nested_attribute", @@ -689,6 +745,62 @@ func TestDataSourceMapNestedAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceMapNestedAttribute{ + Name: "map_nested_attribute", + NestedObject: attrmapper.DataSourceNestedAttributeObject{ + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old nested string description"), + }, + }, + }, + }, + MapNestedAttribute: datasource.MapNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old map nested description"), + }, + }, + mergeAttribute: &attrmapper.DataSourceMapNestedAttribute{ + Name: "map_nested_attribute", + NestedObject: attrmapper.DataSourceNestedAttributeObject{ + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new nested string description"), + }, + }, + }, + }, + MapNestedAttribute: datasource.MapNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new map nested description"), + }, + }, + expectedAttribute: &attrmapper.DataSourceMapNestedAttribute{ + Name: "map_nested_attribute", + NestedObject: attrmapper.DataSourceNestedAttributeObject{ + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old nested string description"), + }, + }, + }, + }, + MapNestedAttribute: datasource.MapNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old map nested description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceMapNestedAttribute{ Name: "map_nested_attribute", diff --git a/internal/mapper/attrmapper/map_test.go b/internal/mapper/attrmapper/map_test.go index 5670676..95b36d6 100644 --- a/internal/mapper/attrmapper/map_test.go +++ b/internal/mapper/attrmapper/map_test.go @@ -114,6 +114,38 @@ func TestResourceMapAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceMapAttribute{ + Name: "map_attribute", + MapAttribute: resource.MapAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old map description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + mergeAttribute: &attrmapper.ResourceMapAttribute{ + Name: "map_attribute", + MapAttribute: resource.MapAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new map description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + expectedAttribute: &attrmapper.ResourceMapAttribute{ + Name: "map_attribute", + MapAttribute: resource.MapAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old map description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceMapAttribute{ Name: "map_attribute", @@ -628,6 +660,38 @@ func TestDataSourceMapAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceMapAttribute{ + Name: "map_attribute", + MapAttribute: datasource.MapAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old map description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + mergeAttribute: &attrmapper.DataSourceMapAttribute{ + Name: "map_attribute", + MapAttribute: datasource.MapAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new map description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + expectedAttribute: &attrmapper.DataSourceMapAttribute{ + Name: "map_attribute", + MapAttribute: datasource.MapAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old map description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceMapAttribute{ Name: "map_attribute", diff --git a/internal/mapper/attrmapper/number.go b/internal/mapper/attrmapper/number.go index 25aecc2..b269da4 100644 --- a/internal/mapper/attrmapper/number.go +++ b/internal/mapper/attrmapper/number.go @@ -24,9 +24,14 @@ func (a *ResourceNumberAttribute) GetName() string { func (a *ResourceNumberAttribute) Merge(mergeAttribute ResourceAttribute) (ResourceAttribute, error) { numberAttribute, ok := mergeAttribute.(*ResourceNumberAttribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = numberAttribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, numberAttribute.ComputedOptionalRequired) return a, nil } @@ -57,9 +62,14 @@ func (a *DataSourceNumberAttribute) GetName() string { func (a *DataSourceNumberAttribute) Merge(mergeAttribute DataSourceAttribute) (DataSourceAttribute, error) { numberAttribute, ok := mergeAttribute.(*DataSourceNumberAttribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = numberAttribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, numberAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/number_test.go b/internal/mapper/attrmapper/number_test.go index aeb5ec6..61b1ce0 100644 --- a/internal/mapper/attrmapper/number_test.go +++ b/internal/mapper/attrmapper/number_test.go @@ -67,6 +67,29 @@ func TestResourceNumberAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceNumberAttribute{ + Name: "number_attribute", + NumberAttribute: resource.NumberAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old number description"), + }, + }, + mergeAttribute: &attrmapper.ResourceNumberAttribute{ + Name: "number_attribute", + NumberAttribute: resource.NumberAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new number description"), + }, + }, + expectedAttribute: &attrmapper.ResourceNumberAttribute{ + Name: "number_attribute", + NumberAttribute: resource.NumberAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old number description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceNumberAttribute{ Name: "number_attribute", @@ -221,6 +244,29 @@ func TestDataSourceNumberAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceNumberAttribute{ + Name: "number_attribute", + NumberAttribute: datasource.NumberAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old number description"), + }, + }, + mergeAttribute: &attrmapper.DataSourceNumberAttribute{ + Name: "number_attribute", + NumberAttribute: datasource.NumberAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new number description"), + }, + }, + expectedAttribute: &attrmapper.DataSourceNumberAttribute{ + Name: "number_attribute", + NumberAttribute: datasource.NumberAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old number description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceNumberAttribute{ Name: "number_attribute", diff --git a/internal/mapper/attrmapper/resource_attributes_test.go b/internal/mapper/attrmapper/resource_attributes_test.go index b9c82d7..ab2a1c0 100644 --- a/internal/mapper/attrmapper/resource_attributes_test.go +++ b/internal/mapper/attrmapper/resource_attributes_test.go @@ -86,6 +86,79 @@ func TestResourceAttributes_Merge(t *testing.T) { }, }, }, + "required in a later merge slice - promote computability": { + targetAttributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "string_attribute", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("string description"), + }, + }, + &attrmapper.ResourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceBoolAttribute{ + Name: "nested_bool", + BoolAttribute: resource.BoolAttribute{ + ComputedOptionalRequired: schema.Computed, + }, + }, + }, + SingleNestedAttribute: resource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + }, + }, + }, + mergeAttributeSlices: []attrmapper.ResourceAttributes{ + { + &attrmapper.ResourceStringAttribute{ + Name: "string_attribute", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("this will be ignored"), + }, + }, + &attrmapper.ResourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceBoolAttribute{ + Name: "nested_bool", + BoolAttribute: resource.BoolAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + SingleNestedAttribute: resource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + }, + expectedAttributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "string_attribute", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("string description"), + }, + }, + &attrmapper.ResourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceBoolAttribute{ + Name: "nested_bool", + BoolAttribute: resource.BoolAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + SingleNestedAttribute: resource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + }, "recursive - matches and appends": { targetAttributes: attrmapper.ResourceAttributes{ &attrmapper.ResourceSingleNestedAttribute{ diff --git a/internal/mapper/attrmapper/set.go b/internal/mapper/attrmapper/set.go index 85cc51e..710d90e 100644 --- a/internal/mapper/attrmapper/set.go +++ b/internal/mapper/attrmapper/set.go @@ -32,6 +32,7 @@ func (a *ResourceSetAttribute) Merge(mergeAttribute ResourceAttribute) (Resource a.Description = setAttribute.Description } a.ElementType = mergeElementType(a.ElementType, setAttribute.ElementType) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, setAttribute.ComputedOptionalRequired) return a, nil } @@ -70,6 +71,7 @@ func (a *DataSourceSetAttribute) Merge(mergeAttribute DataSourceAttribute) (Data a.Description = setAttribute.Description } a.ElementType = mergeElementType(a.ElementType, setAttribute.ElementType) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, setAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/set_nested.go b/internal/mapper/attrmapper/set_nested.go index a3eb1e2..8cd445d 100644 --- a/internal/mapper/attrmapper/set_nested.go +++ b/internal/mapper/attrmapper/set_nested.go @@ -33,6 +33,7 @@ func (a *ResourceSetNestedAttribute) Merge(mergeAttribute ResourceAttribute) (Re a.Description = setNestedAttribute.Description } a.NestedObject.Attributes, _ = a.NestedObject.Attributes.Merge(setNestedAttribute.NestedObject.Attributes) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, setNestedAttribute.ComputedOptionalRequired) return a, nil } @@ -83,6 +84,7 @@ func (a *DataSourceSetNestedAttribute) Merge(mergeAttribute DataSourceAttribute) a.Description = setNestedAttribute.Description } a.NestedObject.Attributes, _ = a.NestedObject.Attributes.Merge(setNestedAttribute.NestedObject.Attributes) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, setNestedAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/set_nested_test.go b/internal/mapper/attrmapper/set_nested_test.go index d8c348d..628d956 100644 --- a/internal/mapper/attrmapper/set_nested_test.go +++ b/internal/mapper/attrmapper/set_nested_test.go @@ -131,6 +131,62 @@ func TestResourceSetNestedAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceSetNestedAttribute{ + Name: "set_nested_attribute", + NestedObject: attrmapper.ResourceNestedAttributeObject{ + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old nested string description"), + }, + }, + }, + }, + SetNestedAttribute: resource.SetNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old set nested description"), + }, + }, + mergeAttribute: &attrmapper.ResourceSetNestedAttribute{ + Name: "set_nested_attribute", + NestedObject: attrmapper.ResourceNestedAttributeObject{ + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new nested string description"), + }, + }, + }, + }, + SetNestedAttribute: resource.SetNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new set nested description"), + }, + }, + expectedAttribute: &attrmapper.ResourceSetNestedAttribute{ + Name: "set_nested_attribute", + NestedObject: attrmapper.ResourceNestedAttributeObject{ + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old nested string description"), + }, + }, + }, + }, + SetNestedAttribute: resource.SetNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old set nested description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceSetNestedAttribute{ Name: "set_nested_attribute", @@ -689,6 +745,62 @@ func TestDataSourceSetNestedAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceSetNestedAttribute{ + Name: "set_nested_attribute", + NestedObject: attrmapper.DataSourceNestedAttributeObject{ + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old nested string description"), + }, + }, + }, + }, + SetNestedAttribute: datasource.SetNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old set nested description"), + }, + }, + mergeAttribute: &attrmapper.DataSourceSetNestedAttribute{ + Name: "set_nested_attribute", + NestedObject: attrmapper.DataSourceNestedAttributeObject{ + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new nested string description"), + }, + }, + }, + }, + SetNestedAttribute: datasource.SetNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new set nested description"), + }, + }, + expectedAttribute: &attrmapper.DataSourceSetNestedAttribute{ + Name: "set_nested_attribute", + NestedObject: attrmapper.DataSourceNestedAttributeObject{ + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old nested string description"), + }, + }, + }, + }, + SetNestedAttribute: datasource.SetNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old set nested description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceSetNestedAttribute{ Name: "set_nested_attribute", diff --git a/internal/mapper/attrmapper/set_test.go b/internal/mapper/attrmapper/set_test.go index b81145e..92e4e93 100644 --- a/internal/mapper/attrmapper/set_test.go +++ b/internal/mapper/attrmapper/set_test.go @@ -114,6 +114,38 @@ func TestResourceSetAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceSetAttribute{ + Name: "set_attribute", + SetAttribute: resource.SetAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old set description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + mergeAttribute: &attrmapper.ResourceSetAttribute{ + Name: "set_attribute", + SetAttribute: resource.SetAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new set description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + expectedAttribute: &attrmapper.ResourceSetAttribute{ + Name: "set_attribute", + SetAttribute: resource.SetAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old set description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceSetAttribute{ Name: "set_attribute", @@ -628,6 +660,38 @@ func TestDataSourceSetAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceSetAttribute{ + Name: "set_attribute", + SetAttribute: datasource.SetAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old set description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + mergeAttribute: &attrmapper.DataSourceSetAttribute{ + Name: "set_attribute", + SetAttribute: datasource.SetAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new set description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + expectedAttribute: &attrmapper.DataSourceSetAttribute{ + Name: "set_attribute", + SetAttribute: datasource.SetAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old set description"), + ElementType: schema.ElementType{ + String: &schema.StringType{}, + }, + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceSetAttribute{ Name: "set_attribute", diff --git a/internal/mapper/attrmapper/single_nested.go b/internal/mapper/attrmapper/single_nested.go index 1571652..6db4688 100644 --- a/internal/mapper/attrmapper/single_nested.go +++ b/internal/mapper/attrmapper/single_nested.go @@ -33,6 +33,7 @@ func (a *ResourceSingleNestedAttribute) Merge(mergeAttribute ResourceAttribute) a.Description = singleNestedAttribute.Description } a.Attributes, _ = a.Attributes.Merge(singleNestedAttribute.Attributes) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, singleNestedAttribute.ComputedOptionalRequired) return a, nil } @@ -81,6 +82,7 @@ func (a *DataSourceSingleNestedAttribute) Merge(mergeAttribute DataSourceAttribu a.Description = singleNestedAttribute.Description } a.Attributes, _ = a.Attributes.Merge(singleNestedAttribute.Attributes) + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, singleNestedAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/single_nested_test.go b/internal/mapper/attrmapper/single_nested_test.go index 0c48ae7..ef53883 100644 --- a/internal/mapper/attrmapper/single_nested_test.go +++ b/internal/mapper/attrmapper/single_nested_test.go @@ -110,6 +110,56 @@ func TestResourceSingleNestedAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old nested string description"), + }, + }, + }, + SingleNestedAttribute: resource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old single nested description"), + }, + }, + mergeAttribute: &attrmapper.ResourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new nested string description"), + }, + }, + }, + SingleNestedAttribute: resource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new single nested description"), + }, + }, + expectedAttribute: &attrmapper.ResourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "nested_string", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old nested string description"), + }, + }, + }, + SingleNestedAttribute: resource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old single nested description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceSingleNestedAttribute{ Name: "single_nested_attribute", @@ -613,6 +663,56 @@ func TestDataSourceSingleNestedAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old nested string description"), + }, + }, + }, + SingleNestedAttribute: datasource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old single nested description"), + }, + }, + mergeAttribute: &attrmapper.DataSourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new nested string description"), + }, + }, + }, + SingleNestedAttribute: datasource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new single nested description"), + }, + }, + expectedAttribute: &attrmapper.DataSourceSingleNestedAttribute{ + Name: "single_nested_attribute", + Attributes: attrmapper.DataSourceAttributes{ + &attrmapper.DataSourceStringAttribute{ + Name: "nested_string", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old nested string description"), + }, + }, + }, + SingleNestedAttribute: datasource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old single nested description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceSingleNestedAttribute{ Name: "single_nested_attribute", diff --git a/internal/mapper/attrmapper/string.go b/internal/mapper/attrmapper/string.go index 9213048..165eef5 100644 --- a/internal/mapper/attrmapper/string.go +++ b/internal/mapper/attrmapper/string.go @@ -24,9 +24,14 @@ func (a *ResourceStringAttribute) GetName() string { func (a *ResourceStringAttribute) Merge(mergeAttribute ResourceAttribute) (ResourceAttribute, error) { stringAttribute, ok := mergeAttribute.(*ResourceStringAttribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = stringAttribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, stringAttribute.ComputedOptionalRequired) return a, nil } @@ -57,9 +62,14 @@ func (a *DataSourceStringAttribute) GetName() string { func (a *DataSourceStringAttribute) Merge(mergeAttribute DataSourceAttribute) (DataSourceAttribute, error) { stringAttribute, ok := mergeAttribute.(*DataSourceStringAttribute) // TODO: return error if types don't match? - if ok && (a.Description == nil || *a.Description == "") { + if !ok { + return a, nil + } + + if a.Description == nil || *a.Description == "" { a.Description = stringAttribute.Description } + a.ComputedOptionalRequired = mergeComputability(a.ComputedOptionalRequired, stringAttribute.ComputedOptionalRequired) return a, nil } diff --git a/internal/mapper/attrmapper/string_test.go b/internal/mapper/attrmapper/string_test.go index b659650..4b51e7b 100644 --- a/internal/mapper/attrmapper/string_test.go +++ b/internal/mapper/attrmapper/string_test.go @@ -67,6 +67,29 @@ func TestResourceStringAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.ResourceStringAttribute{ + Name: "string_attribute", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old string description"), + }, + }, + mergeAttribute: &attrmapper.ResourceStringAttribute{ + Name: "string_attribute", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new string description"), + }, + }, + expectedAttribute: &attrmapper.ResourceStringAttribute{ + Name: "string_attribute", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old string description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.ResourceStringAttribute{ Name: "string_attribute", @@ -221,6 +244,29 @@ func TestDataSourceStringAttribute_Merge(t *testing.T) { }, }, }, + "required merge attribute - promote computability": { + targetAttribute: attrmapper.DataSourceStringAttribute{ + Name: "string_attribute", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Computed, + Description: new("old string description"), + }, + }, + mergeAttribute: &attrmapper.DataSourceStringAttribute{ + Name: "string_attribute", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("new string description"), + }, + }, + expectedAttribute: &attrmapper.DataSourceStringAttribute{ + Name: "string_attribute", + StringAttribute: datasource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("old string description"), + }, + }, + }, "nil description - merge": { targetAttribute: attrmapper.DataSourceStringAttribute{ Name: "string_attribute", diff --git a/internal/mapper/attrmapper/types.go b/internal/mapper/attrmapper/types.go index b9b78bf..a6ce7dd 100644 --- a/internal/mapper/attrmapper/types.go +++ b/internal/mapper/attrmapper/types.go @@ -20,6 +20,22 @@ type ProviderNestedAttributeObject struct { Attributes ProviderAttributes } +// mergeComputability lets a Required classification from the merge attribute override the target's. +// +// Merging is otherwise target-wins, which is correct for everything derived from a schema -- a +// resource's create request body outranks its response bodies. Required is different: it can only +// originate from a parameter the practitioner has to supply, because every response body is built +// with GlobalSchemaOpts.OverrideComputability set to schema.Computed. Discarding it would silently +// demote an attribute the API needs as an input, which is what happened to a read path parameter +// that shared its name with a response property. +func mergeComputability(target schema.ComputedOptionalRequired, merge schema.ComputedOptionalRequired) schema.ComputedOptionalRequired { + if merge == schema.Required { + return schema.Required + } + + return target +} + func mergeElementType(target schema.ElementType, merge schema.ElementType) schema.ElementType { // Handle collection type // https://developer.hashicorp.com/terraform/plugin/framework/handling-data/types#collection-types diff --git a/internal/mapper/datasource_mapper.go b/internal/mapper/datasource_mapper.go index 1627847..f831fc5 100644 --- a/internal/mapper/datasource_mapper.go +++ b/internal/mapper/datasource_mapper.go @@ -15,6 +15,7 @@ import ( "github.com/doitintl/terraform-plugin-codegen-openapi/internal/mapper/util" "github.com/hashicorp/terraform-plugin-codegen-spec/datasource" "github.com/hashicorp/terraform-plugin-codegen-spec/schema" + high "github.com/pb33f/libopenapi/datamodel/high/v3" ) var _ DataSourceMapper = dataSourceMapper{} @@ -107,6 +108,8 @@ func generateDataSourceSchema(logger *slog.Logger, name string, dataSource explo // READ Parameters (optional) // **************** readParameterAttributes := attrmapper.DataSourceAttributes{} + // Keyed by attribute name, i.e. after aliasing, since that is where two parameters collide. + mappedParams := map[string]*high.Parameter{} for _, param := range dataSource.ReadOpParameters() { if param.In != util.OAS_param_path && param.In != util.OAS_param_query { continue @@ -140,6 +143,14 @@ func generateDataSourceSchema(logger *slog.Logger, name string, dataSource explo continue } + // Parameters are identified by name and location, but attributes only by name. Neither + // parameter can win without silently discarding the other, so the specification (or the + // aliases) must be fixed instead. + if mapped, ok := mappedParams[paramName]; ok { + return nil, fmt.Errorf("read operation parameters '%s' (in: %s) and '%s' (in: %s) both map to attribute '%s'", mapped.Name, mapped.In, param.Name, param.In, paramName) + } + mappedParams[paramName] = param + parameterAttribute, schemaErr := s.BuildDataSourceAttribute(paramName, computability) if schemaErr != nil { log.WarnLogOnError(pLogger, schemaErr, "skipping mapping of read operation parameter") diff --git a/internal/mapper/datasource_mapper_test.go b/internal/mapper/datasource_mapper_test.go index 5fac02c..b3504a2 100644 --- a/internal/mapper/datasource_mapper_test.go +++ b/internal/mapper/datasource_mapper_test.go @@ -4,7 +4,9 @@ package mapper_test import ( + "bytes" "log/slog" + "strings" "testing" "github.com/hashicorp/terraform-plugin-codegen-spec/datasource" @@ -840,6 +842,84 @@ func TestDataSourceMapper_basic_merges(t *testing.T) { } } +// Two read parameters that map to the same attribute name have no deterministic merge: keeping +// either one silently discards the other. The data source is rejected so the specification can be fixed. +func TestDataSourceMapper_parameter_name_collisions(t *testing.T) { + t.Parallel() + + stringObject := base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "name": base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + }), + }) + zonePath := &high.Parameter{ + Name: "zone", + In: "path", + Required: new(true), + Schema: base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + } + + testCases := map[string]struct { + readParams []*high.Parameter + schemaOptions explorer.SchemaOptions + wantLog string + }{ + "same name in path and query": { + readParams: []*high.Parameter{ + zonePath, + { + Name: "zone", + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + }, + }, + wantLog: "read operation parameters 'zone' (in: path) and 'zone' (in: query) both map to attribute 'zone'", + }, + "alias onto another parameter's name": { + readParams: []*high.Parameter{ + zonePath, + { + Name: "region", + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + }, + }, + schemaOptions: explorer.SchemaOptions{ + AttributeOptions: explorer.AttributeOptions{ + Aliases: map[string]string{"region": "zone"}, + }, + }, + wantLog: "read operation parameters 'zone' (in: path) and 'region' (in: query) both map to attribute 'zone'", + }, + } + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + var logs bytes.Buffer + mapper := mapper.NewDataSourceMapper(map[string]explorer.DataSource{ + "test_datasource": { + ReadOp: createTestReadOp(stringObject, testCase.readParams), + SchemaOptions: testCase.schemaOptions, + }, + }, config.Config{}) + got, err := mapper.MapToIR(slog.New(slog.NewTextHandler(&logs, nil))) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if len(got) != 0 { + t.Fatalf("expected the data source to be skipped, got: %+v", got) + } + + if !strings.Contains(logs.String(), testCase.wantLog) { + t.Errorf("expected log to contain %q, got: %s", testCase.wantLog, logs.String()) + } + }) + } +} + func TestDataSourceMapper_collections(t *testing.T) { t.Parallel() diff --git a/internal/mapper/resource_mapper.go b/internal/mapper/resource_mapper.go index 2596e15..b39d581 100644 --- a/internal/mapper/resource_mapper.go +++ b/internal/mapper/resource_mapper.go @@ -5,6 +5,7 @@ package mapper import ( "errors" + "fmt" "log/slog" "github.com/doitintl/terraform-plugin-codegen-openapi/internal/config" @@ -15,6 +16,7 @@ import ( "github.com/doitintl/terraform-plugin-codegen-openapi/internal/mapper/util" "github.com/hashicorp/terraform-plugin-codegen-spec/resource" "github.com/hashicorp/terraform-plugin-codegen-spec/schema" + high "github.com/pb33f/libopenapi/datamodel/high/v3" ) var _ ResourceMapper = resourceMapper{} @@ -140,7 +142,21 @@ func generateResourceSchema(logger *slog.Logger, explorerResource explorer.Resou // **************** // READ Parameters (optional) // **************** + // + // A read path parameter that also addresses the create operation is supplied by the + // practitioner, so it maps to Required. One that only appears on the read path identifies + // something the API assigned, so it stays ComputedOptional. `required` alone cannot tell the + // two apart: on /parents/{parent_id}/children/{child_id} both are `required: true`. + createPathParams := map[string]struct{}{} + for _, param := range explorerResource.CreateOpParameters() { + if param.In == util.OAS_param_path { + createPathParams[param.Name] = struct{}{} + } + } + readParameterAttributes := attrmapper.ResourceAttributes{} + // Keyed by attribute name, i.e. after aliasing, since that is where two parameters collide. + mappedParams := map[string]*high.Parameter{} for _, param := range explorerResource.ReadOpParameters() { if param.In != util.OAS_param_path && param.In != util.OAS_param_query { continue @@ -151,6 +167,8 @@ func generateResourceSchema(logger *slog.Logger, explorerResource explorer.Resou Ignores: explorerResource.SchemaOptions.Ignores, OverrideDescription: param.Description, } + // Nested properties of an object-typed parameter are not themselves addressed by the URL, + // so the override stays ComputedOptional even when the parameter itself is Required. globalSchemaOpts := oas.GlobalSchemaOpts{OverrideComputability: schema.ComputedOptional} s, schemaErr := oas.BuildSchema(param.Schema, schemaOpts, globalSchemaOpts) @@ -159,6 +177,13 @@ func generateResourceSchema(logger *slog.Logger, explorerResource explorer.Resou continue } + computability := schema.ComputedOptional + if param.In == util.OAS_param_path && param.Required != nil && *param.Required { + if _, ok := createPathParams[param.Name]; ok { + computability = schema.Required + } + } + // Check for any aliases and replace the paramater name if found paramName := param.Name if aliasedName, ok := explorerResource.SchemaOptions.AttributeOptions.Aliases[param.Name]; ok { @@ -170,7 +195,15 @@ func generateResourceSchema(logger *slog.Logger, explorerResource explorer.Resou continue } - parameterAttribute, schemaErr := s.BuildResourceAttribute(paramName, schema.ComputedOptional) + // Parameters are identified by name and location, but attributes only by name. Neither + // parameter can win without silently discarding the other, so the specification (or the + // aliases) must be fixed instead. + if mapped, ok := mappedParams[paramName]; ok { + return nil, fmt.Errorf("read operation parameters '%s' (in: %s) and '%s' (in: %s) both map to attribute '%s'", mapped.Name, mapped.In, param.Name, param.In, paramName) + } + mappedParams[paramName] = param + + parameterAttribute, schemaErr := s.BuildResourceAttribute(paramName, computability) if schemaErr != nil { log.WarnLogOnError(pLogger, schemaErr, "skipping mapping of read operation parameter") continue diff --git a/internal/mapper/resource_mapper_test.go b/internal/mapper/resource_mapper_test.go index 04a7bd1..8810930 100644 --- a/internal/mapper/resource_mapper_test.go +++ b/internal/mapper/resource_mapper_test.go @@ -4,7 +4,9 @@ package mapper_test import ( + "bytes" "log/slog" + "strings" "testing" "github.com/hashicorp/terraform-plugin-codegen-spec/resource" @@ -29,6 +31,8 @@ func TestResourceMapper_basic_merges(t *testing.T) { createResponseSchema *base.SchemaProxy readResponseSchema *base.SchemaProxy readParams []*high.Parameter + createParams []*high.Parameter + createCommonParams []*high.Parameter schemaOptions explorer.SchemaOptions want resource.Attributes }{ @@ -844,6 +848,336 @@ func TestResourceMapper_basic_merges(t *testing.T) { }, }, }, + "required read path param on the create path - required": { + createRequestSchema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Required: []string{"name"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "name": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }), + }), + createParams: []*high.Parameter{ + { + Name: "parent_id", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + readParams: []*high.Parameter{ + { + Name: "parent_id", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + Name: "child_id", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: resource.Attributes{ + { + Name: "name", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + { + Name: "parent_id", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + { + // Required in the read path but absent from the create path, so the API + // assigned it. + Name: "child_id", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + }, + }, + }, + }, + "create path param declared on the path item - required": { + createRequestSchema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "name": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }), + }), + createCommonParams: []*high.Parameter{ + { + Name: "parent_id", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + readParams: []*high.Parameter{ + { + Name: "parent_id", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: resource.Attributes{ + { + Name: "name", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + }, + }, + { + Name: "parent_id", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + }, + "query params are never promoted": { + createRequestSchema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{}), + }), + createParams: []*high.Parameter{ + { + Name: "zone", + Required: new(true), + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + Name: "region", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + readParams: []*high.Parameter{ + { + // Matches a create query param, not a create path param. + Name: "zone", + Required: new(true), + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + { + // Matches a create path param, but is itself a query param. + Name: "region", + Required: new(true), + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: resource.Attributes{ + { + Name: "zone", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + }, + }, + { + Name: "region", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + }, + }, + }, + }, + "path param not marked required - not promoted": { + createRequestSchema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{}), + }), + createParams: []*high.Parameter{ + { + Name: "parent_id", + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + readParams: []*high.Parameter{ + { + Name: "parent_id", + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: resource.Attributes{ + { + Name: "parent_id", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + }, + }, + }, + }, + "required param overrides a response-derived classification": { + createRequestSchema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{}), + }), + createParams: []*high.Parameter{ + { + Name: "id", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + readResponseSchema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + // The response reaches the merge target first and is forced to Computed, so + // without the promotion the parameter's classification would be discarded. + "id": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + Description: "the response description still wins", + }), + }), + }), + readParams: []*high.Parameter{ + { + Name: "id", + Required: new(true), + In: "path", + Description: "you shouldn't see this, the response was merged first", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: resource.Attributes{ + { + Name: "id", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + Description: new("the response description still wins"), + }, + }, + }, + }, + "required create path param promotes an optional create body property": { + createRequestSchema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + // Optional in the body, but the value is also addressed by the create URL, so + // the practitioner has to supply it and the parameter's Required wins. + "name": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }), + }), + createParams: []*high.Parameter{ + { + Name: "name", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + readParams: []*high.Parameter{ + { + Name: "name", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }, + }, + want: resource.Attributes{ + { + Name: "name", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.Required, + }, + }, + }, + }, + "object-typed required path param keeps nested props computed_optional": { + createRequestSchema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{}), + }), + createParams: []*high.Parameter{ + { + Name: "selector", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + }), + }, + }, + readParams: []*high.Parameter{ + { + Name: "selector", + Required: new(true), + In: "path", + Schema: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Required: []string{"key"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "key": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + }), + }), + }), + }, + }, + want: resource.Attributes{ + { + Name: "selector", + SingleNested: &resource.SingleNestedAttribute{ + ComputedOptionalRequired: schema.Required, + Attributes: resource.Attributes{ + { + Name: "key", + String: &resource.StringAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + }, + }, + }, + }, + }, + }, + }, "ignore bool prop across all ops": { schemaOptions: explorer.SchemaOptions{ Ignores: []string{ @@ -1044,9 +1378,10 @@ func TestResourceMapper_basic_merges(t *testing.T) { mapper := mapper.NewResourceMapper(map[string]explorer.Resource{ "test_resource": { - CreateOp: createTestCreateOp(testCase.createRequestSchema, testCase.createResponseSchema), - ReadOp: createTestReadOp(testCase.readResponseSchema, testCase.readParams), - SchemaOptions: testCase.schemaOptions, + CreateOp: createTestCreateOp(testCase.createRequestSchema, testCase.createResponseSchema, testCase.createParams), + ReadOp: createTestReadOp(testCase.readResponseSchema, testCase.readParams), + CreateCommonParameters: testCase.createCommonParams, + SchemaOptions: testCase.schemaOptions, }, }, config.Config{}) got, err := mapper.MapToIR(slog.Default()) @@ -1065,8 +1400,88 @@ func TestResourceMapper_basic_merges(t *testing.T) { } } -func createTestCreateOp(request *base.SchemaProxy, response *base.SchemaProxy) *high.Operation { +// Two read parameters that map to the same attribute name have no deterministic merge: keeping +// either one silently discards the other. The resource is rejected so the specification can be fixed. +func TestResourceMapper_parameter_name_collisions(t *testing.T) { + t.Parallel() + + stringObject := base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "name": base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + }), + }) + zonePath := &high.Parameter{ + Name: "zone", + In: "path", + Required: new(true), + Schema: base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + } + + testCases := map[string]struct { + readParams []*high.Parameter + schemaOptions explorer.SchemaOptions + wantLog string + }{ + "same name in path and query": { + readParams: []*high.Parameter{ + zonePath, + { + Name: "zone", + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + }, + }, + wantLog: "read operation parameters 'zone' (in: path) and 'zone' (in: query) both map to attribute 'zone'", + }, + "alias onto another parameter's name": { + readParams: []*high.Parameter{ + zonePath, + { + Name: "region", + In: "query", + Schema: base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + }, + }, + schemaOptions: explorer.SchemaOptions{ + AttributeOptions: explorer.AttributeOptions{ + Aliases: map[string]string{"region": "zone"}, + }, + }, + wantLog: "read operation parameters 'zone' (in: path) and 'region' (in: query) both map to attribute 'zone'", + }, + } + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + var logs bytes.Buffer + mapper := mapper.NewResourceMapper(map[string]explorer.Resource{ + "test_resource": { + CreateOp: createTestCreateOp(stringObject, nil, []*high.Parameter{zonePath}), + ReadOp: createTestReadOp(stringObject, testCase.readParams), + SchemaOptions: testCase.schemaOptions, + }, + }, config.Config{}) + got, err := mapper.MapToIR(slog.New(slog.NewTextHandler(&logs, nil))) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if len(got) != 0 { + t.Fatalf("expected the resource to be skipped, got: %+v", got) + } + + if !strings.Contains(logs.String(), testCase.wantLog) { + t.Errorf("expected log to contain %q, got: %s", testCase.wantLog, logs.String()) + } + }) + } +} + +func createTestCreateOp(request *base.SchemaProxy, response *base.SchemaProxy, params []*high.Parameter) *high.Operation { return &high.Operation{ + Parameters: params, RequestBody: &high.RequestBody{ Content: orderedmap.ToOrderedMap(map[string]*high.MediaType{ "application/json": {