From ad2f2ad3423e4f15fdfc0d73fe39a2dda9d30e10 Mon Sep 17 00:00:00 2001 From: Murad Biashimov Date: Fri, 17 Jan 2025 11:19:54 +0100 Subject: [PATCH] refactor: use ResourceData interface Replaces `*schema.ResourceData` type with `schemautil.ResourceData` in the shared code. The interface allows to write tests with mockery. --- internal/schemautil/common.go | 2 +- internal/schemautil/resourcedata.go | 2 + internal/schemautil/schemautil.go | 10 ++-- internal/schemautil/service.go | 10 ++-- internal/schemautil/static_ips.go | 2 +- internal/schemautil/wait.go | 16 +++---- .../userconfig/converters/converters.go | 18 +++---- .../userconfig/converters/utils.go | 8 +++- .../userconfig/converters/utils_test.go | 16 +++++++ mocks/resourcedata.go | 48 +++++++++++++++++++ 10 files changed, 102 insertions(+), 30 deletions(-) diff --git a/internal/schemautil/common.go b/internal/schemautil/common.go index 51a136cf0..8fd03fcab 100644 --- a/internal/schemautil/common.go +++ b/internal/schemautil/common.go @@ -57,7 +57,7 @@ func SetTagsTerraformProperties(t map[string]string) []map[string]interface{} { return tags } -func GetTagsFromSchema(d *schema.ResourceData) map[string]string { +func GetTagsFromSchema(d ResourceData) map[string]string { tags := make(map[string]string) for _, tag := range d.Get("tag").(*schema.Set).List() { diff --git a/internal/schemautil/resourcedata.go b/internal/schemautil/resourcedata.go index 418935e6e..d7f441ca6 100644 --- a/internal/schemautil/resourcedata.go +++ b/internal/schemautil/resourcedata.go @@ -2,6 +2,7 @@ package schemautil import ( "context" + "time" avngen "github.com/aiven/go-client-codegen" "github.com/hashicorp/go-cty/cty" @@ -21,6 +22,7 @@ type ResourceData interface { GetOk(string) (any, bool) GetRawConfig() cty.Value HasChange(string) bool + Timeout(string) time.Duration } // WithResourceData same as common.WithGenClient, except it takes ResourceData instead of *schema.ResourceData diff --git a/internal/schemautil/schemautil.go b/internal/schemautil/schemautil.go index 5435beaa9..3766e6116 100644 --- a/internal/schemautil/schemautil.go +++ b/internal/schemautil/schemautil.go @@ -17,7 +17,7 @@ import ( // OptionalStringPointer retrieves a string pointer to a field, empty string // will be converted to nil -func OptionalStringPointer(d *schema.ResourceData, key string) *string { +func OptionalStringPointer(d ResourceData, key string) *string { val, ok := d.GetOk(key) if !ok { return nil @@ -30,7 +30,7 @@ func OptionalStringPointer(d *schema.ResourceData, key string) *string { } // OptionalIntPointer retrieves an int pointer to a field, if the field is not set, returns nil. -func OptionalIntPointer(d *schema.ResourceData, key string) *int { +func OptionalIntPointer(d ResourceData, key string) *int { val, ok := d.GetOk(key) if !ok { return nil @@ -43,7 +43,7 @@ func OptionalIntPointer(d *schema.ResourceData, key string) *int { } // OptionalBoolPointer retrieves a bool pointer to a field, if the field is not set, returns nil. -func OptionalBoolPointer(d *schema.ResourceData, key string) *bool { +func OptionalBoolPointer(d ResourceData, key string) *bool { val, ok := d.GetOk(key) if !ok { return nil @@ -256,7 +256,7 @@ func FlattenToString[T any](a []T) []string { } func CopyServiceUserPropertiesFromAPIResponseToTerraform( - d *schema.ResourceData, + d ResourceData, user *aiven.ServiceUser, projectName string, serviceName string, @@ -292,7 +292,7 @@ func CopyServiceUserPropertiesFromAPIResponseToTerraform( } func CopyServiceUserGenPropertiesFromAPIResponseToTerraform( - d *schema.ResourceData, + d ResourceData, user *service.ServiceUserGetOut, projectName string, serviceName string, diff --git a/internal/schemautil/service.go b/internal/schemautil/service.go index 08927042d..0d879290f 100644 --- a/internal/schemautil/service.go +++ b/internal/schemautil/service.go @@ -682,7 +682,7 @@ func ResourceServiceDelete(ctx context.Context, d *schema.ResourceData, m interf } func copyServicePropertiesFromAPIResponseToTerraform( - d *schema.ResourceData, + d ResourceData, s *service.ServiceGetOut, servicePlanParams PlanParameters, project string, @@ -844,7 +844,7 @@ func FlattenServiceComponents(r *service.ServiceGetOut) []map[string]interface{} // // We should change this in the next major version. func copyConnectionInfoFromAPIResponseToTerraform( - d *schema.ResourceData, + d ResourceData, serviceType string, connectionInfo *service.ConnectionInfoOut, metadata map[string]any, @@ -1057,7 +1057,7 @@ func DatasourceServiceRead(ctx context.Context, d *schema.ResourceData, m interf return diag.Errorf("common %s/%s not found", projectName, serviceName) } -func getContactEmailListForAPI(d *schema.ResourceData) (*[]service.TechEmailIn, error) { +func getContactEmailListForAPI(d ResourceData) (*[]service.TechEmailIn, error) { if valuesInterface, ok := d.GetOk("tech_emails"); ok { var emails []service.TechEmailIn err := Remarshal(valuesInterface.(*schema.Set).List(), &emails) @@ -1069,11 +1069,11 @@ func getContactEmailListForAPI(d *schema.ResourceData) (*[]service.TechEmailIn, return &[]service.TechEmailIn{}, nil } -func ExpandService(name string, d *schema.ResourceData) (map[string]any, error) { +func ExpandService(name string, d ResourceData) (map[string]any, error) { return converters.Expand(converters.ServiceUserConfig, name, d) } -func FlattenService(name string, d *schema.ResourceData, dto map[string]any) error { +func FlattenService(name string, d ResourceData, dto map[string]any) error { return converters.Flatten(converters.ServiceUserConfig, name, d, dto) } diff --git a/internal/schemautil/static_ips.go b/internal/schemautil/static_ips.go index 0ffb79726..958d884af 100644 --- a/internal/schemautil/static_ips.go +++ b/internal/schemautil/static_ips.go @@ -43,7 +43,7 @@ func ServiceStaticIpsList(ctx context.Context, client avngen.Client, projectName } // DiffStaticIps takes a service resource and computes which static ips to assign and which to disassociate -func DiffStaticIps(ctx context.Context, d *schema.ResourceData, client avngen.Client) (ass, dis []string, err error) { +func DiffStaticIps(ctx context.Context, d ResourceData, client avngen.Client) (ass, dis []string, err error) { ipsFromSchema := FlattenToString(d.Get("static_ips").(*schema.Set).List()) ipsFromAPI, err := ServiceStaticIpsList(ctx, client, d.Get("project").(string), d.Get("service_name").(string)) if err != nil { diff --git a/internal/schemautil/wait.go b/internal/schemautil/wait.go index e2b43b6f4..6fa40c33d 100644 --- a/internal/schemautil/wait.go +++ b/internal/schemautil/wait.go @@ -27,7 +27,7 @@ const ( aivenServicesStartingState = "WAITING_FOR_SERVICES" ) -func WaitForServiceCreation(ctx context.Context, d *schema.ResourceData, client avngen.Client) (*service.ServiceGetOut, error) { +func WaitForServiceCreation(ctx context.Context, d ResourceData, client avngen.Client) (*service.ServiceGetOut, error) { projectName, serviceName := d.Get("project").(string), d.Get("service_name").(string) timeout := d.Timeout(schema.TimeoutCreate) @@ -80,7 +80,7 @@ func WaitForServiceCreation(ctx context.Context, d *schema.ResourceData, client return aux.(*service.ServiceGetOut), nil } -func WaitForServiceUpdate(ctx context.Context, d *schema.ResourceData, client avngen.Client) (*service.ServiceGetOut, error) { +func WaitForServiceUpdate(ctx context.Context, d ResourceData, client avngen.Client) (*service.ServiceGetOut, error) { projectName, serviceName := d.Get("project").(string), d.Get("service_name").(string) timeout := d.Timeout(schema.TimeoutUpdate) @@ -129,7 +129,7 @@ func WaitForServiceUpdate(ctx context.Context, d *schema.ResourceData, client av return aux.(*service.ServiceGetOut), nil } -func WaitStaticIpsDissociation(ctx context.Context, d *schema.ResourceData, m interface{}) error { +func WaitStaticIpsDissociation(ctx context.Context, d ResourceData, m interface{}) error { timeout := d.Timeout(schema.TimeoutDelete) log.Printf("[DEBUG] Static Ip dissassociation timeout %.0f minutes", timeout.Minutes()) @@ -158,7 +158,7 @@ func WaitStaticIpsDissociation(ctx context.Context, d *schema.ResourceData, m in return nil } -func WaitForDeletion(ctx context.Context, d *schema.ResourceData, m interface{}) error { +func WaitForDeletion(ctx context.Context, d ResourceData, m interface{}) error { client := m.(*aiven.Client) projectName, serviceName := d.Get("project").(string), d.Get("service_name").(string) @@ -272,7 +272,7 @@ func backupsReady(s *service.ServiceGetOut) bool { // staticIpsReady checks that the static ips that are associated with the service are either // in state 'assigned' or 'available' -func staticIpsReady(ctx context.Context, d *schema.ResourceData, client avngen.Client) (bool, error) { +func staticIpsReady(ctx context.Context, d ResourceData, client avngen.Client) (bool, error) { resourceIPs := staticIpsForServiceFromSchema(d) if len(resourceIPs) == 0 { return true, nil @@ -300,7 +300,7 @@ func staticIpsReady(ctx context.Context, d *schema.ResourceData, client avngen.C // all static ips that were associated to the service are available again func staticIpsDisassociatedAfterServiceDeletion( ctx context.Context, - d *schema.ResourceData, + d ResourceData, m interface{}, ) (bool, error) { expectedStaticIps := staticIpsForServiceFromSchema(d) @@ -333,7 +333,7 @@ func staticIpsDisassociatedAfterServiceDeletion( // staticIpsAreDisassociated checks that after service update // all static ips that are not used by the service anymore are available again -func staticIpsAreDisassociated(ctx context.Context, d *schema.ResourceData, m interface{}) (bool, error) { +func staticIpsAreDisassociated(ctx context.Context, d ResourceData, m interface{}) (bool, error) { client := m.(*aiven.Client) projectName := d.Get("project").(string) serviceName := d.Get("service_name").(string) @@ -359,7 +359,7 @@ L: return true, nil } -func staticIpsForServiceFromSchema(d *schema.ResourceData) []string { +func staticIpsForServiceFromSchema(d ResourceData) []string { return FlattenToString(d.Get("static_ips").(*schema.Set).List()) } diff --git a/internal/sdkprovider/userconfig/converters/converters.go b/internal/sdkprovider/userconfig/converters/converters.go index 3eb48463a..47c10fb3b 100644 --- a/internal/sdkprovider/userconfig/converters/converters.go +++ b/internal/sdkprovider/userconfig/converters/converters.go @@ -87,7 +87,7 @@ func SetUserConfig(kind userConfigType, name string, s map[string]*schema.Schema s[userConfigKey(kind, name)] = userConfig } -func Expand(kind userConfigType, name string, d *schema.ResourceData) (map[string]any, error) { +func Expand(kind userConfigType, name string, d resourceData) (map[string]any, error) { m, err := expand(kind, name, d) if err != nil { return nil, fmt.Errorf("error converting user config options for %s type %q to API format: %w", kind, name, err) @@ -97,7 +97,7 @@ func Expand(kind userConfigType, name string, d *schema.ResourceData) (map[strin // expand expands schema.ResourceData into a DTO map. // It takes schema.Schema to know how to turn a TF item into json. -func expand(kind userConfigType, name string, d *schema.ResourceData) (map[string]any, error) { +func expand(kind userConfigType, name string, d resourceData) (map[string]any, error) { if getUserConfig(kind, name) == nil { // does not have a user config for given kind and name return nil, nil @@ -143,11 +143,11 @@ func expand(kind userConfigType, name string, d *schema.ResourceData) (map[strin // With the state it is possible to say "if value is null", hence if it is defined by user. // With schema.ResourceData, you get the value that might be a zero-value. type stateCompose struct { - key string // state attribute name or schema.ResourceData key - path string // schema.ResourceData path, i.e., foo.0.bar.0.baz to get the value - schema *schema.Schema // tf schema - config cty.Value // tf file state, it knows if resource value is null - resource *schema.ResourceData // tf resource that has both tf state and data that is received from the API + key string // state attribute name or schema.ResourceData key + path string // schema.ResourceData path, i.e., foo.0.bar.0.baz to get the value + schema *schema.Schema // tf schema + config cty.Value // tf file state, it knows if resource value is null + resource resourceData // tf resource that has both tf state and data that is received from the API } // listItems returns a list of object's states @@ -343,7 +343,7 @@ func expandList(s *stateCompose) (any, error) { return items, nil } -func Flatten(kind userConfigType, name string, d *schema.ResourceData, dto map[string]any) error { +func Flatten(kind userConfigType, name string, d resourceData, dto map[string]any) error { err := flatten(kind, name, d, dto) if err != nil { return fmt.Errorf("error converting user config options for %s type %q from API format: %w", kind, name, err) @@ -352,7 +352,7 @@ func Flatten(kind userConfigType, name string, d *schema.ResourceData, dto map[s } // flatten flattens DTO into a terraform compatible object and sets value to the field -func flatten(kind userConfigType, name string, d *schema.ResourceData, dto map[string]any) error { +func flatten(kind userConfigType, name string, d resourceData, dto map[string]any) error { if getUserConfig(kind, name) == nil { // does not have a user config for given kind and name return nil diff --git a/internal/sdkprovider/userconfig/converters/utils.go b/internal/sdkprovider/userconfig/converters/utils.go index c6a558d30..d3d9a9e04 100644 --- a/internal/sdkprovider/userconfig/converters/utils.go +++ b/internal/sdkprovider/userconfig/converters/utils.go @@ -6,6 +6,7 @@ import ( "reflect" "strings" + "github.com/hashicorp/go-cty/cty" "golang.org/x/exp/maps" "golang.org/x/exp/slices" ) @@ -30,9 +31,14 @@ func renameAliasesToDto(kind userConfigType, name string, dto map[string]any) { } } -// resourceData to test schema.ResourceData with unit tests +// resourceData implements *schema.ResourceData type resourceData interface { + Get(string) any GetOk(string) (any, bool) + GetRawConfig() cty.Value + HasChange(string) bool + IsNewResource() bool + Set(string, any) error } // renameAliasesToTfo renames aliases to TF object diff --git a/internal/sdkprovider/userconfig/converters/utils_test.go b/internal/sdkprovider/userconfig/converters/utils_test.go index e8466052d..36909284f 100644 --- a/internal/sdkprovider/userconfig/converters/utils_test.go +++ b/internal/sdkprovider/userconfig/converters/utils_test.go @@ -5,6 +5,7 @@ import ( "regexp" "testing" + "github.com/hashicorp/go-cty/cty" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -249,6 +250,21 @@ type resourceDataMock struct { m map[string]any } +func (r *resourceDataMock) Get(_ string) any { + panic("implement me") +} +func (r *resourceDataMock) GetRawConfig() cty.Value { + panic("implement me") +} +func (r *resourceDataMock) HasChange(_ string) bool { + panic("implement me") +} +func (r *resourceDataMock) IsNewResource() bool { + panic("implement me") +} +func (r *resourceDataMock) Set(_ string, _ any) error { + panic("implement me") +} func (r *resourceDataMock) GetOk(k string) (any, bool) { v, ok := r.m[k] return v, ok diff --git a/mocks/resourcedata.go b/mocks/resourcedata.go index 3eee11564..9e5ed0891 100644 --- a/mocks/resourcedata.go +++ b/mocks/resourcedata.go @@ -5,6 +5,8 @@ package mocks import ( cty "github.com/hashicorp/go-cty/cty" mock "github.com/stretchr/testify/mock" + + time "time" ) // MockResourceData is an autogenerated mock type for the ResourceData type @@ -387,6 +389,52 @@ func (_c *MockResourceData_SetId_Call) RunAndReturn(run func(string)) *MockResou return _c } +// Timeout provides a mock function with given fields: _a0 +func (_m *MockResourceData) Timeout(_a0 string) time.Duration { + ret := _m.Called(_a0) + + if len(ret) == 0 { + panic("no return value specified for Timeout") + } + + var r0 time.Duration + if rf, ok := ret.Get(0).(func(string) time.Duration); ok { + r0 = rf(_a0) + } else { + r0 = ret.Get(0).(time.Duration) + } + + return r0 +} + +// MockResourceData_Timeout_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Timeout' +type MockResourceData_Timeout_Call struct { + *mock.Call +} + +// Timeout is a helper method to define mock.On call +// - _a0 string +func (_e *MockResourceData_Expecter) Timeout(_a0 interface{}) *MockResourceData_Timeout_Call { + return &MockResourceData_Timeout_Call{Call: _e.mock.On("Timeout", _a0)} +} + +func (_c *MockResourceData_Timeout_Call) Run(run func(_a0 string)) *MockResourceData_Timeout_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(string)) + }) + return _c +} + +func (_c *MockResourceData_Timeout_Call) Return(_a0 time.Duration) *MockResourceData_Timeout_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockResourceData_Timeout_Call) RunAndReturn(run func(string) time.Duration) *MockResourceData_Timeout_Call { + _c.Call.Return(run) + return _c +} + // NewMockResourceData creates a new instance of MockResourceData. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewMockResourceData(t interface {