Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove attribute and block schema templates #99

Merged
merged 42 commits into from
Jan 9, 2024

Conversation

bendbennett
Copy link
Contributor

@bendbennett bendbennett commented Dec 3, 2023

This PR is a follow-up to Consolidate convert and generate packages for datasource, provider, and resource.

The intention is to remove usage of attribute and block schema templates (e.g., bool_attribute.gotmpl, set_nested_block.gotmpl), and to replace with calls to reusable attribute, and block field methods (e.g., ComputedOptionalRequired.Schema()).

Previous implementation of the Schema() method on GeneratorBoolAttribute looked as follows:

func (g GeneratorBoolAttribute) Schema(name generatorschema.FrameworkIdentifier) (string, error) {
	type attribute struct {
		Name                   string
		CustomType             string
		GeneratorBoolAttribute GeneratorBoolAttribute
	}

	a := attribute{
		Name:                   name.ToString(),
		GeneratorBoolAttribute: g,
	}

	switch {
	case g.CustomType != nil:
		a.CustomType = g.CustomType.Type
	case g.AssociatedExternalType != nil:
		a.CustomType = fmt.Sprintf("%sType{}", name.ToPascalCase())
	}

	t, err := template.New("bool_attribute").Parse(boolAttributeTemplate)

	if err != nil {
		return "", err
	}

	if _, err = addAttributeTemplate(t); err != nil {
		return "", err
	}

	var buf strings.Builder

	err = t.Execute(&buf, a)
	if err != nil {
		return "", err
	}

	return buf.String(), nil
}

The Schema() method on GeneratorBoolAttribute now appears as:

func (g GeneratorBoolAttribute) Schema(name generatorschema.FrameworkIdentifier) (string, error) {
	var b bytes.Buffer

	b.WriteString(fmt.Sprintf("%q: schema.BoolAttribute{\n", name))
	b.Write(g.AttributeType.Schema())
	b.Write(g.ComputedOptionalRequired.Schema())
	b.Write(g.Sensitive.Schema())
	b.Write(g.Description.Schema())
	b.Write(g.DeprecationMessage.Schema())
	b.Write(g.ValidatorsCustom.Schema())
	b.WriteString("},")

	return b.String(), nil
}

Further Refactoring

With the changes in this PR, new types (e.g., CustomTypePrimitive) have been added. At this point, the terraform-plugin-codegen-spec CustomType, is held in more than one location within the Generator<Type>Attributes. For example:

type GeneratorBoolAttribute struct {
	AssociatedExternalType   *generatorschema.AssocExtType
	ComputedOptionalRequired convert.ComputedOptionalRequired
	CustomType               *specschema.CustomType
	CustomTypePrimitive      convert.CustomTypePrimitive
	/* ... */     

The CustomType field will be refactored out of GeneratorBoolAttribute as the functions that make use of this field (e.g., Imports(), ModelField()) are refactored to use CustomTypePrimitive.

Equivalent changes will be made for all fields across the various generator types during subsequent refactoring.

@bendbennett bendbennett added this to the v0.4.0 milestone Dec 3, 2023
Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

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

This all looks good to me, if you want to continue with resource/data source and the other types, I'm good with merging this 🚀

@@ -96,46 +113,22 @@ func (g GeneratorBoolAttribute) Equal(ga generatorschema.GeneratorAttribute) boo
return false
}

return g.BoolAttribute.Equal(h.BoolAttribute)
return true
}

func (g GeneratorBoolAttribute) Schema(name generatorschema.FrameworkIdentifier) (string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Much cleaner and easier to read! I'm a big fan 😄

Base automatically changed from consolidate-convert-and-generate to main December 14, 2023 07:25
Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

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

sweet! at a high-level these changes all LGTM. It will be nice to be able to place debug breakpoints in the middle of all this code generation if we need to 😄

@bendbennett bendbennett merged commit 41c2efe into main Jan 9, 2024
4 checks passed
@bendbennett bendbennett deleted the remove-schema-templates branch January 9, 2024 10:13
Copy link

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 22, 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