From fc9e4c8143f2838512d0503215a13fa7b3c5ebbd Mon Sep 17 00:00:00 2001 From: Miles Yucht Date: Thu, 12 Dec 2024 11:02:07 +0100 Subject: [PATCH] tests and better error --- .../pluginfw/tfschema/attribute_converter.go | 6 + .../tfschema/customizable_schema_test.go | 272 +++++++----------- .../tfschema/single_nested_attribute.go | 2 +- .../pluginfw/tfschema/single_nested_block.go | 16 -- 4 files changed, 114 insertions(+), 182 deletions(-) diff --git a/internal/providers/pluginfw/tfschema/attribute_converter.go b/internal/providers/pluginfw/tfschema/attribute_converter.go index d76760660..8611509c0 100644 --- a/internal/providers/pluginfw/tfschema/attribute_converter.go +++ b/internal/providers/pluginfw/tfschema/attribute_converter.go @@ -21,6 +21,12 @@ func convertAttributesToBlocks(attributes map[string]AttributeBuilder, blocks ma for name, block := range blocks { newBlocks[name] = block } + if len(newAttributes) == 0 { + newAttributes = nil + } + if len(newBlocks) == 0 { + newBlocks = nil + } return NestedBlockObject{ Attributes: newAttributes, Blocks: newBlocks, diff --git a/internal/providers/pluginfw/tfschema/customizable_schema_test.go b/internal/providers/pluginfw/tfschema/customizable_schema_test.go index 12d8f44ad..1dfec1772 100644 --- a/internal/providers/pluginfw/tfschema/customizable_schema_test.go +++ b/internal/providers/pluginfw/tfschema/customizable_schema_test.go @@ -263,168 +263,110 @@ func (m mockValidator) ValidateObject(context.Context, validator.ObjectRequest, var _ validator.List = mockValidator{} var _ validator.Object = mockValidator{} -// func TestCustomizeSchema_ConvertToAttribute(t *testing.T) { -// v := mockValidator{} -// pm := mockPlanModifier{} -// testCases := []struct { -// name string -// baseSchema NestedBlockObject -// path []string -// want NestedBlockObject -// expectPanic bool -// }{ -// { -// name: "ListNestedBlock", -// baseSchema: NestedBlockObject{ -// Blocks: map[string]BlockBuilder{ -// "list": ListNestedBlockBuilder{ -// NestedObject: NestedBlockObject{ -// Attributes: map[string]AttributeBuilder{ -// "attr": StringAttributeBuilder{}, -// }, -// }, -// DeprecationMessage: "deprecated", -// Validators: []validator.List{v}, -// PlanModifiers: []planmodifier.List{pm}, -// }, -// }, -// }, -// path: []string{"list"}, -// want: NestedBlockObject{ -// Attributes: map[string]AttributeBuilder{ -// "list": ListNestedAttributeBuilder{ -// NestedObject: NestedAttributeObject{ -// Attributes: map[string]AttributeBuilder{ -// "attr": StringAttributeBuilder{}, -// }, -// }, -// DeprecationMessage: "deprecated", -// Validators: []validator.List{v}, -// PlanModifiers: []planmodifier.List{pm}, -// }, -// }, -// }, -// }, -// { -// name: "ListNestedBlock/CalledOnInnerBlock", -// baseSchema: NestedBlockObject{ -// Blocks: map[string]BlockBuilder{ -// "list": ListNestedBlockBuilder{ -// NestedObject: NestedBlockObject{ -// Blocks: map[string]BlockBuilder{ -// "nested_block": ListNestedBlockBuilder{ -// NestedObject: NestedBlockObject{ -// Attributes: map[string]AttributeBuilder{ -// "attr": StringAttributeBuilder{}, -// }, -// }, -// }, -// }, -// }, -// }, -// }, -// }, -// path: []string{"list", "nested_block"}, -// want: NestedBlockObject{ -// Blocks: map[string]BlockBuilder{ -// "list": ListNestedBlockBuilder{ -// NestedObject: NestedBlockObject{ -// Attributes: map[string]AttributeBuilder{ -// "nested_block": ListNestedAttributeBuilder{ -// NestedObject: NestedAttributeObject{ -// Attributes: map[string]AttributeBuilder{ -// "attr": StringAttributeBuilder{}, -// }, -// }, -// }, -// }, -// }, -// }, -// }, -// }, -// }, -// { -// name: "SingleNestedBlock", -// baseSchema: NestedBlockObject{ -// Blocks: map[string]BlockBuilder{ -// "single": SingleNestedBlockBuilder{ -// NestedObject: NestedBlockObject{ -// Attributes: map[string]AttributeBuilder{ -// "attr": StringAttributeBuilder{}, -// }, -// }, -// DeprecationMessage: "deprecated", -// Validators: []validator.Object{v}, -// PlanModifiers: []planmodifier.Object{pm}, -// }, -// }, -// }, -// path: []string{"single"}, -// want: NestedBlockObject{ -// Attributes: map[string]AttributeBuilder{ -// "single": SingleNestedAttributeBuilder{ -// Attributes: map[string]AttributeBuilder{ -// "attr": StringAttributeBuilder{}, -// }, -// DeprecationMessage: "deprecated", -// Validators: []validator.Object{v}, -// PlanModifiers: []planmodifier.Object{pm}, -// }, -// }, -// }, -// }, -// { -// name: "SingleNestedBlock/RecursiveBlocks", -// baseSchema: NestedBlockObject{ -// Blocks: map[string]BlockBuilder{ -// "single": SingleNestedBlockBuilder{ -// NestedObject: NestedBlockObject{ -// Blocks: map[string]BlockBuilder{ -// "nested_block": ListNestedBlockBuilder{ -// NestedObject: NestedBlockObject{ -// Attributes: map[string]AttributeBuilder{ -// "attr": StringAttributeBuilder{}, -// }, -// }, -// }, -// }, -// }, -// }, -// }, -// }, -// path: []string{"single"}, -// want: NestedBlockObject{ -// Attributes: map[string]AttributeBuilder{ -// "single": SingleNestedAttributeBuilder{ -// Attributes: map[string]AttributeBuilder{ -// "nested_block": ListNestedAttributeBuilder{ -// NestedObject: NestedAttributeObject{ -// Attributes: map[string]AttributeBuilder{ -// "attr": StringAttributeBuilder{}, -// }, -// }, -// }, -// }, -// }, -// }, -// }, -// }, -// { -// name: "PanicOnEmptyPath", -// path: nil, -// expectPanic: true, -// }, -// } -// for _, c := range testCases { -// t.Run(c.name, func(t *testing.T) { -// if c.expectPanic { -// assert.Panics(t, func() { -// ConstructCustomizableSchema(c.baseSchema).ConvertToAttribute(c.path...) -// }) -// } else { -// got := ConstructCustomizableSchema(c.baseSchema).ConvertToAttribute(c.path...) -// assert.Equal(t, c.want, got.attr.(SingleNestedBlockBuilder).NestedObject) -// } -// }) -// } -// } +func TestCustomizeSchema_ConfigureForSdkV2Migration(t *testing.T) { + v := mockValidator{} + pm := mockPlanModifier{} + testCases := []struct { + name string + baseSchema NestedBlockObject + want NestedBlockObject + expectPanic bool + }{ + { + name: "ListNestedAttribute", + baseSchema: NestedBlockObject{ + Attributes: map[string]AttributeBuilder{ + "list": ListNestedAttributeBuilder{ + NestedObject: NestedAttributeObject{ + Attributes: map[string]AttributeBuilder{ + "attr": StringAttributeBuilder{}, + }, + }, + DeprecationMessage: "deprecated", + Validators: []validator.List{v}, + PlanModifiers: []planmodifier.List{pm}, + }, + }, + }, + want: NestedBlockObject{ + Blocks: map[string]BlockBuilder{ + "list": ListNestedBlockBuilder{ + NestedObject: NestedBlockObject{ + Attributes: map[string]AttributeBuilder{ + "attr": StringAttributeBuilder{}, + }, + }, + DeprecationMessage: "deprecated", + Validators: []validator.List{v}, + PlanModifiers: []planmodifier.List{pm}, + }, + }, + }, + }, + { + name: "ListNestedAttribute/RecursiveBlocks", + baseSchema: NestedBlockObject{ + Attributes: map[string]AttributeBuilder{ + "list": ListNestedAttributeBuilder{ + NestedObject: NestedAttributeObject{ + Attributes: map[string]AttributeBuilder{ + "nested_block": ListNestedAttributeBuilder{ + NestedObject: NestedAttributeObject{ + Attributes: map[string]AttributeBuilder{ + "attr": StringAttributeBuilder{}, + }, + }, + }, + }, + }, + }, + }, + }, + want: NestedBlockObject{ + Blocks: map[string]BlockBuilder{ + "list": ListNestedBlockBuilder{ + NestedObject: NestedBlockObject{ + Blocks: map[string]BlockBuilder{ + "nested_block": ListNestedBlockBuilder{ + NestedObject: NestedBlockObject{ + Attributes: map[string]AttributeBuilder{ + "attr": StringAttributeBuilder{}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "SingleNestedBlock/Panics", + baseSchema: NestedBlockObject{ + Attributes: map[string]AttributeBuilder{ + "single": SingleNestedAttributeBuilder{ + Attributes: map[string]AttributeBuilder{ + "attr": StringAttributeBuilder{}, + }, + DeprecationMessage: "deprecated", + Validators: []validator.Object{v}, + PlanModifiers: []planmodifier.Object{pm}, + }, + }, + }, + expectPanic: true, + }, + } + for _, c := range testCases { + t.Run(c.name, func(t *testing.T) { + if c.expectPanic { + assert.Panics(t, func() { + ConstructCustomizableSchema(c.baseSchema).ConfigureForSdkV2Migration() + }) + } else { + got := ConstructCustomizableSchema(c.baseSchema).ConfigureForSdkV2Migration() + assert.Equal(t, c.want, got.attr.(SingleNestedBlockBuilder).NestedObject) + } + }) + } +} diff --git a/internal/providers/pluginfw/tfschema/single_nested_attribute.go b/internal/providers/pluginfw/tfschema/single_nested_attribute.go index 62fe04bdc..0321d0c37 100644 --- a/internal/providers/pluginfw/tfschema/single_nested_attribute.go +++ b/internal/providers/pluginfw/tfschema/single_nested_attribute.go @@ -107,5 +107,5 @@ func (a SingleNestedAttributeBuilder) AddPlanModifier(v planmodifier.Object) Att } func (a SingleNestedAttributeBuilder) ToBlock() BlockBuilder { - panic(fmt.Errorf("SingleNestedAttributeBuilder used for legacy resource. This is unexpected. %s", common.TerraformBugErrorMessage)) + panic(fmt.Errorf("ToBlock() called on SingleNestedAttributeBuilder. This means that the corresponding field is a types.Object, which should never happen for legacy resources. %s", common.TerraformBugErrorMessage)) } diff --git a/internal/providers/pluginfw/tfschema/single_nested_block.go b/internal/providers/pluginfw/tfschema/single_nested_block.go index 59508235b..1b88c9ae0 100644 --- a/internal/providers/pluginfw/tfschema/single_nested_block.go +++ b/internal/providers/pluginfw/tfschema/single_nested_block.go @@ -68,19 +68,3 @@ func (a SingleNestedBlockBuilder) AddPlanModifier(v planmodifier.Object) BaseSch a.PlanModifiers = append(a.PlanModifiers, v) return a } - -func (a SingleNestedBlockBuilder) ConvertBlockToAttribute(field string) BaseSchemaBuilder { - elem, ok := a.NestedObject.Blocks[field] - if !ok { - panic(fmt.Errorf("field %s does not exist in nested block", field)) - } - if a.NestedObject.Attributes == nil { - a.NestedObject.Attributes = make(map[string]AttributeBuilder) - } - a.NestedObject.Attributes[field] = elem.ToAttribute() - delete(a.NestedObject.Blocks, field) - if len(a.NestedObject.Blocks) == 0 { - a.NestedObject.Blocks = nil - } - return a -}