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

all: Add support for embedded struct types in object conversions #1021

Merged
merged 17 commits into from
Aug 2, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor to use Tag.Lookup and add tests for empty tags
austinvalle committed Aug 1, 2024
commit 94de60dcdf39db14aad965bf3107a89e043d59c2
72 changes: 38 additions & 34 deletions internal/reflect/helpers.go
Original file line number Diff line number Diff line change
@@ -79,50 +79,54 @@ func getStructTags(ctx context.Context, typ reflect.Type, path path.Path) (map[s
// This index sequence is the location of the field within the struct.
// For promoted fields from an embedded struct, the length of this sequence will be > 1
fieldIndexSequence := []int{i}
tag := field.Tag.Get(`tfsdk`)

switch tag {
// "tfsdk" tags can only be omitted on embedded structs
case "":
if field.Anonymous {
embeddedTags, err := getStructTags(ctx, field.Type, path)
if err != nil {
return nil, fmt.Errorf(`error retrieving embedded struct %q field tags: %w`, field.Name, err)
}
for k, v := range embeddedTags {
if other, ok := tags[k]; ok {
otherField := typ.FieldByIndex(other)
return nil, fmt.Errorf("embedded struct %q promotes a field with a duplicate tfsdk tag %q, conflicts with %q tfsdk tag", field.Name, k, otherField.Name)
}

tags[k] = append(fieldIndexSequence, v...)
}
continue
}

return nil, fmt.Errorf(`%s: need a struct tag for "tfsdk" on %s`, path, field.Name)
tag, tagExists := field.Tag.Lookup(`tfsdk`)

// "tfsdk" tags with "-" are being explicitly excluded
case "-":
if tag == "-" {
continue
}

// validate the "tfsdk" tag and ensure there are no duplicates before storing
default:
path := path.AtName(tag)
if field.Anonymous {
return nil, fmt.Errorf(`%s: embedded struct field %s cannot have tfsdk tag`, path, field.Name)
// Handle embedded structs
if field.Anonymous {
if tagExists {
return nil, fmt.Errorf(`%s: embedded struct field %s cannot have tfsdk tag`, path.AtName(tag), field.Name)
}
if !isValidFieldName(tag) {
return nil, fmt.Errorf("%s: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter", path)

embeddedTags, err := getStructTags(ctx, field.Type, path)
if err != nil {
return nil, fmt.Errorf(`error retrieving embedded struct %q field tags: %w`, field.Name, err)
}
if other, ok := tags[tag]; ok {
otherField := typ.FieldByIndex(other)
return nil, fmt.Errorf("%s: can't use tfsdk tag %q for both %s and %s fields", path, tag, otherField.Name, field.Name)
for k, v := range embeddedTags {
if other, ok := tags[k]; ok {
otherField := typ.FieldByIndex(other)
return nil, fmt.Errorf("embedded struct %q promotes a field with a duplicate tfsdk tag %q, conflicts with %q tfsdk tag", field.Name, k, otherField.Name)
}

tags[k] = append(fieldIndexSequence, v...)
}
continue
}

// All non-embedded fields must have a tfsdk tag
if !tagExists {
return nil, fmt.Errorf(`%s: need a struct tag for "tfsdk" on %s`, path, field.Name)
}

tags[tag] = fieldIndexSequence
// Ensure the tfsdk tag has a valid name
path := path.AtName(tag)
if !isValidFieldName(tag) {
return nil, fmt.Errorf("%s: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter", path)
}

// Ensure there are no duplicate tfsdk tags
if other, ok := tags[tag]; ok {
otherField := typ.FieldByIndex(other)
return nil, fmt.Errorf("%s: can't use tfsdk tag %q for both %s and %s fields", path, tag, otherField.Name, field.Name)
}

tags[tag] = fieldIndexSequence
}

return tags, nil
}

24 changes: 24 additions & 0 deletions internal/reflect/helpers_test.go
Original file line number Diff line number Diff line change
@@ -84,6 +84,18 @@ func TestGetStructTags(t *testing.T) {
in: StructWithInvalidTag{},
expectedErr: errors.New(`*()-: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter`),
},
"struct-err-missing-tfsdk-tag": {
in: struct {
ExampleField string
}{},
expectedErr: errors.New(`: need a struct tag for "tfsdk" on ExampleField`),
},
"struct-err-empty-tfsdk-tag": {
in: struct {
ExampleField string `tfsdk:""`
}{},
expectedErr: errors.New(`: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter`),
},
"ignore-embedded-struct": {
in: struct {
ExampleStruct `tfsdk:"-"`
@@ -134,6 +146,12 @@ func TestGetStructTags(t *testing.T) {
"field5": {1},
},
},
"embedded-struct-err-cannot-have-empty-tfsdk-tag": {
in: struct {
ExampleStruct `tfsdk:""` // Can't put a tfsdk tag here
}{},
expectedErr: errors.New(`: embedded struct field ExampleStruct cannot have tfsdk tag`),
},
"embedded-struct-err-cannot-have-tfsdk-tag": {
in: struct {
ExampleStruct `tfsdk:"example_field"` // Can't put a tfsdk tag here
@@ -185,6 +203,12 @@ func TestGetStructTags(t *testing.T) {
"field5": {1},
},
},
"embedded-struct-ptr-err-cannot-have-empty-tfsdk-tag": {
in: struct {
*ExampleStruct `tfsdk:""` // Can't put a tfsdk tag here
}{},
expectedErr: errors.New(`: embedded struct field ExampleStruct cannot have tfsdk tag`),
},
"embedded-struct-ptr-err-cannot-have-tfsdk-tag": {
in: struct {
*ExampleStruct `tfsdk:"example_field"` // Can't put a tfsdk tag here
82 changes: 82 additions & 0 deletions internal/reflect/struct_test.go
Original file line number Diff line number Diff line change
@@ -223,6 +223,26 @@ func TestNewStruct_errors(t *testing.T) {
"error retrieving field names from struct tags: %w",
errors.New("invalidTag: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter")),
},
"struct-has-empty-tag": {
typ: types.ObjectType{
AttrTypes: map[string]attr.Type{
"a": types.StringType,
},
},
objVal: tftypes.NewValue(tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"a": tftypes.String,
},
}, map[string]tftypes.Value{
"a": tftypes.NewValue(tftypes.String, "hello"),
}),
targetVal: reflect.ValueOf(struct {
A string `tfsdk:""`
}{}),
expectedError: fmt.Errorf(
"error retrieving field names from struct tags: %w",
errors.New(": invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter")),
},
"embedded-struct-has-invalid-tags": {
typ: types.ObjectType{
AttrTypes: map[string]attr.Type{
@@ -332,6 +352,26 @@ func TestNewStruct_errors(t *testing.T) {
"Error: reflect: indirection through nil pointer to embedded struct field embedSingleField",
),
},
"embedded-struct-has-empty-tfsdk-tag": {
typ: types.ObjectType{
AttrTypes: map[string]attr.Type{
"attr_1": types.StringType,
},
},
objVal: tftypes.NewValue(tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"attr_1": tftypes.String,
},
}, map[string]tftypes.Value{
"attr_1": tftypes.NewValue(tftypes.String, "hello"),
}),
targetVal: reflect.ValueOf(struct {
embedSingleField `tfsdk:""`
}{}),
expectedError: errors.New(
"error retrieving field names from struct tags: : embedded struct field embedSingleField cannot have tfsdk tag",
),
},
"embedded-struct-has-tfsdk-tag": {
typ: types.ObjectType{
AttrTypes: map[string]attr.Type{
@@ -1803,6 +1843,27 @@ func TestFromStruct_errors(t *testing.T) {
),
},
},
"struct-has-empty-tag": {
typ: types.ObjectType{
AttrTypes: map[string]attr.Type{
"test": types.StringType,
},
},
val: reflect.ValueOf(
struct {
Test types.String `tfsdk:""`
}{},
),
expectedDiags: diag.Diagnostics{
diag.NewAttributeErrorDiagnostic(
path.Root("test"),
"Value Conversion Error",
"An unexpected error was encountered trying to convert from struct value. "+
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
"error retrieving field names from struct tags: test.: invalid tfsdk tag, must only use lowercase letters, underscores, and numbers, and must start with a letter",
),
},
},
"embedded-struct-has-invalid-tags": {
typ: types.ObjectType{
AttrTypes: map[string]attr.Type{
@@ -2016,6 +2077,27 @@ func TestFromStruct_errors(t *testing.T) {
),
},
},
"embedded-struct-has-empty-tfsdk-tag": {
typ: types.ObjectType{
AttrTypes: map[string]attr.Type{
"attr_1": types.StringType,
},
},
val: reflect.ValueOf(
struct {
embedSingleField `tfsdk:""`
}{},
),
expectedDiags: diag.Diagnostics{
diag.NewAttributeErrorDiagnostic(
path.Root("test"),
"Value Conversion Error",
"An unexpected error was encountered trying to convert from struct value. "+
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+
"error retrieving field names from struct tags: test.: embedded struct field embedSingleField cannot have tfsdk tag",
),
},
},
"embedded-struct-has-tfsdk-tag": {
typ: types.ObjectType{
AttrTypes: map[string]attr.Type{