Skip to content

Refactor merging + add abstraction for attributes#51

Merged
austinvalle merged 16 commits into
mainfrom
av/override
Aug 25, 2023
Merged

Refactor merging + add abstraction for attributes#51
austinvalle merged 16 commits into
mainfrom
av/override

Conversation

@austinvalle

@austinvalle austinvalle commented Aug 23, 2023

Copy link
Copy Markdown
Member

Recent PR discussion: #50 (comment)

This PR introduces a new package that wraps the codegen-spec types to allow our code to easily handle generic logic such as: merging, overriding, etc. This PR is all internal refactoring and no new features/oddities have been added.

Prior art considered: https://github.com/hashicorp/terraform-plugin-codegen-framework/tree/main/internal/resource_generate

Notes

  • For nested objects + nested attributes, we shadow the original NestedObject and Attributes properties. It looks a little weird when creating a struct literal, but otherwise behaves normally:
singleNestedAttribute := ResourceAttributes{
	&ResourceSingleNestedAttribute{
		Name: "single_nested_attribute",
		// This is correct!
		Attributes: ResourceAttributes{
			&ResourceStringAttribute{
				Name: "string_attribute",
				StringAttribute: resource.StringAttribute{
					ComputedOptionalRequired: schema.Required,
				},
			},
		},
		SingleNestedAttribute: resource.SingleNestedAttribute{
			// This is being shadowed!
			// Attributes: resource.Attributes{},
			ComputedOptionalRequired: schema.Required,
		},
	},
}
  • Element types don't have any considerations during merging/overriding, so I didn't create wrappers for them and just kept the original function for merging:
    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
    if target.List != nil && merge.List != nil {
    target.List.ElementType = mergeElementType(target.List.ElementType, merge.List.ElementType)
    } else if target.Map != nil && merge.Map != nil {
    target.Map.ElementType = mergeElementType(target.Map.ElementType, merge.Map.ElementType)
    } else if target.Set != nil && merge.Set != nil {
    target.Set.ElementType = mergeElementType(target.Set.ElementType, merge.Set.ElementType)
    }
    // Handle object type
    // https://developer.hashicorp.com/terraform/plugin/framework/handling-data/types#object-type
    if target.Object != nil && merge.Object != nil {
    target.Object.AttributeTypes = mergeObjectAttributeTypes(target.Object.AttributeTypes, merge.Object.AttributeTypes)
    }
    return target
    }

@austinvalle austinvalle marked this pull request as ready for review August 23, 2023 20:57
@austinvalle austinvalle requested a review from a team as a code owner August 23, 2023 20:57

@bflad bflad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 🚀

Comment thread internal/mapper/attrmapper/attributes.go
Comment thread internal/mapper/attrmapper/attributes.go Outdated
@github-actions

Copy link
Copy Markdown

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants