Add custom type handling for collection elements and object attribute types #9

bendbennett · 2023-06-06T15:59:54Z

Within the terraform-plugin-codegen-spec repository the JSON schema and the Go bindings for the schema define the intermediate representation.

The schema and Go bindings allow for the usage of custom types in conjunction with element types within collections (i.e., list, map, set) and attribute types within objects.

The changes in this PR include the following:

Handling of custom types for collection element types and object attribute types in the schema generation for data sources, provider and resources.
Handling of custom types for collection element types and object attribute types in the generation of import declarations for use in conjunction with the generated schema for data sources, provider and resources.

… (private repo)

…d spec generation

…ource

…esources

…import is included in the top-level schema.gotmpl

… object attribute types

…ObjectAttributeType

… and resource

bflad

Overall I think this really looks good. Just a few little things to potentially followup on. 😄

bflad · 2023-06-09T16:23:47Z

internal/schema/schema.go

+		return "types.Int64Type"
+	case e.List != nil:
+		if e.List.CustomType != nil {
+			return fmt.Sprintf("%s{\nElemType: %s,\n}", e.List.CustomType.Type, GetElementType(e.List.ElementType))


This is okay as a placeholder for now, but we may need a way in the specification to ensure that developers can fully specify how this should be created as code since its not guaranteed that the custom type will be implemented with an ElemType field. Similarly with object types.

I've created 2 issues for this:

Consider providing a way to fully specify custom type (codegen-spec).

Fully specify custom type (codegen-framework follow-up issue).

bflad · 2023-06-09T16:25:03Z

internal/schema/schema.go

+		}
+		return "types.NumberType"
+	case e.Object != nil:
+		return fmt.Sprintf("types.ObjectType{\nAttrTypes: map[string]attr.Type{\n%s\n},\n}", GetAttrTypes(e.Object))


Should this also be implementing e.Object.CustomType logic similar to the collection types?

e.Object is a []ObjectAttributeType but there's no e.Object.CustomType available.

Perhaps this needs to be added to the spec?

Ah ha, yes it would 👍

I've added an issue for Add CustomType to the Specification Go Bindings.

bflad · 2023-06-09T16:30:27Z

internal/schema/schema.go

+		switch {
+		case v.Bool != nil:
+			if v.Bool.CustomType != nil {
+				aTypes.WriteString(fmt.Sprintf("\"%s\": %s,", v.Name, v.Bool.CustomType.Type))


Nit: These can use %q to remove the \"%s\" handling. It may also be worth moving the v.Name writing logic above the switch since branches require the logic.

aTypes.WriteString(fmt.Sprintf("%q: ", v.Name)) switch { case v.Bool != nil: if v.Bool.CustomType != nil { aTypes.WriteString(v.Bool.CustomType.Type) } else { aTypes.WriteString("types.BoolType") } // ... } aTypes.WriteString(",")

(We could also consider making a "schema definition string" method on each of the types so calling logic doesn't need to handle the implementation details like the switch statement here 😄 )

I've added an issue to Consider adding schema definition string method on types.

bflad · 2023-06-20T13:28:50Z

internal/datasource_generate/list_attribute.go

 )

 type GeneratorListAttribute struct {
 	schema.ListAttribute

-	CustomType *specschema.CustomType
-	Validators []specschema.ListValidator
+	CustomType  *specschema.CustomType


Nit: It might be good to throw a quick comment in these generator types about why we're preferring the "specschema" types here, rather than using the "real" schema types -- e.g. because we need to support extracting custom import information later. 👍

bflad · 2023-06-20T13:43:03Z

internal/provider_generate/generator.go

+	}
+
+	for a := range imports {
+		s.WriteString(fmt.Sprintf("\"%s\"\n", a))


Nit: 😎

Suggested change

s.WriteString(fmt.Sprintf("\"%s\"\n", a))

s.WriteString(fmt.Sprintf("%q\n", a))

bflad · 2023-06-20T14:05:08Z

internal/resource_generate/generator.go

+	}
+
+	for a := range imports {
+		s.WriteString(fmt.Sprintf("\"%s\"\n", a))


Nit: 😎

Suggested change

s.WriteString(fmt.Sprintf("\"%s\"\n", a))

s.WriteString(fmt.Sprintf("%q\n", a))

bflad · 2023-06-20T14:06:18Z

internal/resource_generate/generator.go

@@ -198,3 +230,93 @@ func customTypeEqual(x, y *specschema.CustomType) bool {

 	return true
 }
+
+func objectTypeEqual(x, y []specschema.ObjectAttributeType) bool {


Would you mind raising a feature request issue for consideration of this in terraform-plugin-codegen-spec? Thanks!

I've created an issue for Consider adding equality functions in the terraform-plugin-codegen-spec repo.

bflad

Looks good to me 🚀

github-actions · 2024-05-23T22:18:24Z

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.

bendbennett added 15 commits June 6, 2023 15:40

Incorporating usage of spec element type

a75888c

Adding tests for custom types on elements

88cfa48

Updating ci-go.yml to use token for pulling modules from codegen-spec…

8eca8e4

… (private repo)

Linting

f616f4e

Merge branch 'main' into schema-provider-resources

f685238

Using latest version of main and calling spec.Parse for validation an…

ea638cc

…d spec generation

Switching to using HasImport available in spec

237dd34

Add handling of element type and attribute type into provider and res…

a006f7a

…ource

Add handling of default and plan modifier imports into resources

d880e52

Switching to using HasImport() helper method

aa76ba8

Removing unused function

b337d8c

Wiring-up addition of imports into schema template for provider and r…

4c3a606

…esources

Removing unneeded schemaImport inclusion in import generation as the …

8226f2e

…import is included in the top-level schema.gotmpl

Moving commands to separate directory

c3863f1

Adding new line when object contains more than one attribute type

a592e5d

bendbennett mentioned this pull request Jun 8, 2023

Add validation to ensure duplicate names are not present within the schema #2

Closed

bendbennett added 3 commits June 8, 2023 14:50

Adding copyright headers

ec90ac6

Handling custom type in schema generation for collection elements and…

d734561

… object attribute types

Amending golangci configuration to skip output directory

61a3099

bendbennett marked this pull request as ready for review June 9, 2023 06:28

bendbennett requested a review from a team as a code owner June 9, 2023 06:28

bendbennett changed the title ~~[WIP] Add custom type handling for elements and schema generation for provider and resources~~ Add custom type handling for collection elements and object attribute types Jun 9, 2023

Refactoring to used shared functions for spec schema ElementType and …

0654f06

…ObjectAttributeType

austinvalle mentioned this pull request Jun 16, 2023

Add simple e2e tests for all command #15

Merged

bendbennett added 3 commits June 19, 2023 13:51

Merge branch 'main' into schema-provider-resources

ff9a2be

Updating package names to match module name

9401282

Removing unneeded import from schema.gotmpl for data source, provider…

4d1a19c

… and resource

bflad reviewed Jun 20, 2023

View reviewed changes

bendbennett added 2 commits June 20, 2023 16:07

Adding nil check in template for custom field in plan modifier

4933849

Adding nil check in template for custom field in validator

3aff33b

bendbennett added 3 commits June 20, 2023 17:25

DRYing up usage of WriteString()

acf6776

Adding comment about the usage specschema types

8ca1800

Using string quoting within fmt.Sprintf()

e8f2a3b

bflad approved these changes Jun 20, 2023

View reviewed changes

bendbennett merged commit b404a8c into main Jun 21, 2023

bendbennett deleted the schema-provider-resources branch June 21, 2023 06:59

github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom type handling for collection elements and object attribute types #9

Add custom type handling for collection elements and object attribute types #9

bendbennett commented Jun 6, 2023 •

edited

Loading

bflad left a comment

bflad Jun 9, 2023

bendbennett Jun 20, 2023

bflad Jun 9, 2023

bendbennett Jun 20, 2023

bflad Jun 20, 2023

bendbennett Jun 20, 2023

bflad Jun 9, 2023

bflad Jun 20, 2023 •

edited

Loading

bendbennett Jun 20, 2023

bflad Jun 20, 2023

bflad Jun 20, 2023

bflad Jun 20, 2023

bflad Jun 20, 2023

bendbennett Jun 20, 2023

bflad left a comment

github-actions bot commented May 23, 2024

	s.WriteString(fmt.Sprintf("\"%s\"\n", a))
	s.WriteString(fmt.Sprintf("%q\n", a))

Add custom type handling for collection elements and object attribute types #9

Add custom type handling for collection elements and object attribute types #9

Conversation

bendbennett commented Jun 6, 2023 • edited Loading

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bflad Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bflad left a comment

Choose a reason for hiding this comment

github-actions bot commented May 23, 2024

bendbennett commented Jun 6, 2023 •

edited

Loading

bflad Jun 20, 2023 •

edited

Loading