From 78b01d6287fb817d36d177a5cda2642302533915 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Thu, 7 Dec 2023 12:49:51 -0600 Subject: [PATCH] refactor: move resource data change detectors to separate package --- equinix/provider.go | 55 ---------- equinix/provider_test.go | 128 +----------------------- equinix/resource_ecx_l2_connection.go | 5 +- equinix/resource_metal_organization.go | 2 +- equinix/resource_network_device.go | 7 +- equinix/resource_network_device_link.go | 14 ++- internal/schema/resourcedata.go | 46 +++++++++ internal/schema/resourcedata_test.go | 106 ++++++++++++++++++++ internal/schema/set_map.go | 2 +- 9 files changed, 175 insertions(+), 190 deletions(-) create mode 100644 internal/schema/resourcedata.go create mode 100644 internal/schema/resourcedata_test.go diff --git a/equinix/provider.go b/equinix/provider.go index 0f12f62bd..7f1f8185a 100644 --- a/equinix/provider.go +++ b/equinix/provider.go @@ -3,7 +3,6 @@ package equinix import ( "context" "fmt" - "reflect" "strings" "time" @@ -24,15 +23,6 @@ var ( NetworkTypeListHB = strings.Join(DeviceNetworkTypesHB, ", ") ) -// resourceDataProvider provies interface to schema.ResourceData -// for convenient mocking purposes -type resourceDataProvider interface { - Get(key string) interface{} - GetOk(key string) (interface{}, bool) - HasChange(key string) bool - GetChange(key string) (interface{}, interface{}) -} - // Provider returns Equinix terraform *schema.Provider func Provider() *schema.Provider { provider := &schema.Provider{ @@ -249,40 +239,6 @@ func isStringInSlice(needle string, hay []string) bool { return false } -func getResourceDataChangedKeys(keys []string, d resourceDataProvider) map[string]interface{} { - changed := make(map[string]interface{}) - for _, key := range keys { - if v := d.Get(key); v != nil && d.HasChange(key) { - changed[key] = v - } - } - return changed -} - -func getResourceDataListElementChanges(keys []string, listKeyName string, listIndex int, d resourceDataProvider) map[string]interface{} { - changed := make(map[string]interface{}) - if !d.HasChange(listKeyName) { - return changed - } - old, new := d.GetChange(listKeyName) - oldList := old.([]interface{}) - newList := new.([]interface{}) - if len(oldList) < listIndex || len(newList) < listIndex { - return changed - } - return getMapChangedKeys(keys, oldList[listIndex].(map[string]interface{}), newList[listIndex].(map[string]interface{})) -} - -func getMapChangedKeys(keys []string, old, new map[string]interface{}) map[string]interface{} { - changed := make(map[string]interface{}) - for _, key := range keys { - if !reflect.DeepEqual(old[key], new[key]) { - changed[key] = new[key] - } - } - return changed -} - func isEmpty(v interface{}) bool { switch v := v.(type) { case int: @@ -347,14 +303,3 @@ func slicesMatchCaseInsensitive(s1, s2 []string) bool { } return true } - -func schemaSetToMap(set *schema.Set) map[int]interface{} { - transformed := make(map[int]interface{}) - if set != nil { - list := set.List() - for i := range list { - transformed[set.F(list[i])] = list[i] - } - } - return transformed -} diff --git a/equinix/provider_test.go b/equinix/provider_test.go index e70ea2a3d..7809e9cd7 100644 --- a/equinix/provider_test.go +++ b/equinix/provider_test.go @@ -3,15 +3,12 @@ package equinix import ( "fmt" "os" - "reflect" "regexp" "strings" "testing" - "github.com/equinix/terraform-provider-equinix/internal/config" - "github.com/equinix/terraform-provider-equinix/internal/hashcode" - "github.com/equinix/ecx-go/v2" + "github.com/equinix/terraform-provider-equinix/internal/config" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/stretchr/testify/assert" @@ -24,28 +21,6 @@ var ( testExternalProviders map[string]resource.ExternalProvider ) -type mockedResourceDataProvider struct { - actual map[string]interface{} - old map[string]interface{} -} - -func (r mockedResourceDataProvider) Get(key string) interface{} { - return r.actual[key] -} - -func (r mockedResourceDataProvider) GetOk(key string) (interface{}, bool) { - v, ok := r.actual[key] - return v, ok -} - -func (r mockedResourceDataProvider) HasChange(key string) bool { - return !reflect.DeepEqual(r.old[key], r.actual[key]) -} - -func (r mockedResourceDataProvider) GetChange(key string) (interface{}, interface{}) { - return r.old[key], r.actual[key] -} - type mockECXClient struct { GetUserPortsFn func() ([]ecx.Port, error) @@ -176,82 +151,6 @@ func TestProvider_stringsFound_negative(t *testing.T) { assert.False(t, result, "Given strings were found") } -func TestProvider_resourceDataChangedKeys(t *testing.T) { - // given - keys := []string{"key", "keyTwo", "keyThree"} - rd := mockedResourceDataProvider{ - actual: map[string]interface{}{ - "key": "value", - "keyTwo": "newValueTwo", - }, - old: map[string]interface{}{ - "key": "value", - "keyTwo": "valueTwo", - }, - } - expected := map[string]interface{}{ - "keyTwo": "newValueTwo", - } - // when - result := getResourceDataChangedKeys(keys, rd) - // then - assert.Equal(t, expected, result, "Function returns valid key changes") -} - -func TestProvider_resourceDataListElementChanges(t *testing.T) { - // given - keys := []string{"key", "keyTwo", "keyThree"} - listKeyName := "myList" - rd := mockedResourceDataProvider{ - old: map[string]interface{}{ - listKeyName: []interface{}{ - map[string]interface{}{ - "key": "value", - "keyTwo": "valueTwo", - "keyThree": 50, - }, - }, - }, - actual: map[string]interface{}{ - listKeyName: []interface{}{ - map[string]interface{}{ - "key": "value", - "keyTwo": "newValueTwo", - "keyThree": 100, - }, - }, - }, - } - expected := map[string]interface{}{ - "keyTwo": "newValueTwo", - "keyThree": 100, - } - // when - result := getResourceDataListElementChanges(keys, listKeyName, 0, rd) - // then - assert.Equal(t, expected, result, "Function returns valid key changes") -} - -func TestProvider_mapChanges(t *testing.T) { - // given - keys := []string{"key", "keyTwo", "keyThree"} - old := map[string]interface{}{ - "key": "value", - "keyTwo": "valueTwo", - } - new := map[string]interface{}{ - "key": "newValue", - "keyTwo": "valueTwo", - } - expected := map[string]interface{}{ - "key": "newValue", - } - // when - result := getMapChangedKeys(keys, old, new) - // then - assert.Equal(t, expected, result, "Function returns valid key changes") -} - func TestProvider_isEmpty(t *testing.T) { // given input := []interface{}{ @@ -331,31 +230,6 @@ func TestProvider_slicesMatch(t *testing.T) { } } -func TestProvider_schemaSetToMap(t *testing.T) { - // given - type item struct { - id string - valueOne int - valueTwo int - } - setFunc := func(v interface{}) int { - i := v.(item) - return hashcode.String(i.id) - } - items := []interface{}{ - item{"id1", 100, 200}, - item{"id2", 666, 999}, - item{"id3", 0, 100}, - } - set := schema.NewSet(setFunc, items) - // when - list := schemaSetToMap(set) - // then - assert.Equal(t, items[0], list[setFunc(items[0])]) - assert.Equal(t, items[1], list[setFunc(items[1])]) - assert.Equal(t, items[2], list[setFunc(items[2])]) -} - //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ // Test helper functions //_______________________________________________________________________ diff --git a/equinix/resource_ecx_l2_connection.go b/equinix/resource_ecx_l2_connection.go index 20d20af70..8db6f778f 100644 --- a/equinix/resource_ecx_l2_connection.go +++ b/equinix/resource_ecx_l2_connection.go @@ -10,6 +10,7 @@ import ( "github.com/equinix/terraform-provider-equinix/internal/config" "github.com/equinix/terraform-provider-equinix/internal/converters" equinix_errors "github.com/equinix/terraform-provider-equinix/internal/errors" + equinix_schema "github.com/equinix/terraform-provider-equinix/internal/schema" equinix_validation "github.com/equinix/terraform-provider-equinix/internal/validation" "github.com/equinix/ecx-go/v2" @@ -755,13 +756,13 @@ func resourceECXL2ConnectionUpdate(ctx context.Context, d *schema.ResourceData, ecxL2ConnectionSchemaNames["Speed"], ecxL2ConnectionSchemaNames["SpeedUnit"], } - primaryChanges := getResourceDataChangedKeys(supportedChanges, d) + primaryChanges := equinix_schema.GetResourceDataChangedKeys(supportedChanges, d) primaryUpdateReq := client.NewL2ConnectionUpdateRequest(d.Id()) if err := fillFabricL2ConnectionUpdateRequest(primaryUpdateReq, primaryChanges).Execute(); err != nil { return diag.FromErr(err) } if redID, ok := d.GetOk(ecxL2ConnectionSchemaNames["RedundantUUID"]); ok { - secondaryChanges := getResourceDataListElementChanges(supportedChanges, ecxL2ConnectionSchemaNames["SecondaryConnection"], 0, d) + secondaryChanges := equinix_schema.GetResourceDataListElementChanges(supportedChanges, ecxL2ConnectionSchemaNames["SecondaryConnection"], 0, d) secondaryUpdateReq := client.NewL2ConnectionUpdateRequest(redID.(string)) if err := fillFabricL2ConnectionUpdateRequest(secondaryUpdateReq, secondaryChanges).Execute(); err != nil { return diag.FromErr(err) diff --git a/equinix/resource_metal_organization.go b/equinix/resource_metal_organization.go index d51dedf75..deeed8059 100644 --- a/equinix/resource_metal_organization.go +++ b/equinix/resource_metal_organization.go @@ -174,7 +174,7 @@ func resourceMetalOrganizationUpdate(d *schema.ResourceData, meta interface{}) e meta.(*config.Config).AddModuleToMetalUserAgent(d) client := meta.(*config.Config).Metal - changes := getResourceDataChangedKeys([]string{"name", "description", "website", "twitter", "logo", "address"}, d) + changes := equinix_schema.GetResourceDataChangedKeys([]string{"name", "description", "website", "twitter", "logo", "address"}, d) updateRequest := &packngo.OrganizationUpdateRequest{} for change, changeValue := range changes { switch change { diff --git a/equinix/resource_network_device.go b/equinix/resource_network_device.go index 31cdc5d38..07b21033f 100644 --- a/equinix/resource_network_device.go +++ b/equinix/resource_network_device.go @@ -11,6 +11,7 @@ import ( "github.com/equinix/terraform-provider-equinix/internal/config" "github.com/equinix/terraform-provider-equinix/internal/converters" + equinix_schema "github.com/equinix/terraform-provider-equinix/internal/schema" equinix_validation "github.com/equinix/terraform-provider-equinix/internal/validation" "github.com/equinix/ne-go" @@ -965,11 +966,11 @@ func resourceNetworkDeviceUpdate(ctx context.Context, d *schema.ResourceData, m neDeviceSchemaNames["ACLTemplateUUID"], neDeviceSchemaNames["MgmtAclTemplateUuid"], } updateReq := client.NewDeviceUpdateRequest(d.Id()) - primaryChanges := getResourceDataChangedKeys(supportedChanges, d) + primaryChanges := equinix_schema.GetResourceDataChangedKeys(supportedChanges, d) var clusterChanges map[string]interface{} clusterSupportedChanges := []string{neDeviceClusterSchemaNames["ClusterName"]} if _, ok := d.GetOk(neDeviceSchemaNames["ClusterDetails"]); ok { - clusterChanges = getResourceDataListElementChanges(clusterSupportedChanges, neDeviceSchemaNames["ClusterDetails"], 0, d) + clusterChanges = equinix_schema.GetResourceDataListElementChanges(clusterSupportedChanges, neDeviceSchemaNames["ClusterDetails"], 0, d) for key, value := range clusterChanges { primaryChanges[key] = value } @@ -979,7 +980,7 @@ func resourceNetworkDeviceUpdate(ctx context.Context, d *schema.ResourceData, m } var secondaryChanges map[string]interface{} if v, ok := d.GetOk(neDeviceSchemaNames["RedundantUUID"]); ok { - secondaryChanges = getResourceDataListElementChanges(supportedChanges, neDeviceSchemaNames["Secondary"], 0, d) + secondaryChanges = equinix_schema.GetResourceDataListElementChanges(supportedChanges, neDeviceSchemaNames["Secondary"], 0, d) secondaryUpdateReq := client.NewDeviceUpdateRequest(v.(string)) if err := fillNetworkDeviceUpdateRequest(secondaryUpdateReq, secondaryChanges).Execute(); err != nil { return diag.FromErr(err) diff --git a/equinix/resource_network_device_link.go b/equinix/resource_network_device_link.go index ec59cb482..cdb417ed3 100644 --- a/equinix/resource_network_device_link.go +++ b/equinix/resource_network_device_link.go @@ -8,6 +8,7 @@ import ( "github.com/equinix/terraform-provider-equinix/internal/config" equinix_errors "github.com/equinix/terraform-provider-equinix/internal/errors" "github.com/equinix/terraform-provider-equinix/internal/hashcode" + equinix_schema "github.com/equinix/terraform-provider-equinix/internal/schema" equinix_validation "github.com/equinix/terraform-provider-equinix/internal/validation" "github.com/equinix/ne-go" @@ -274,7 +275,7 @@ func resourceNetworkDeviceLinkUpdate(ctx context.Context, d *schema.ResourceData client := m.(*config.Config).Ne m.(*config.Config).AddModuleToNEUserAgent(&client, d) var diags diag.Diagnostics - changes := getResourceDataChangedKeys([]string{ + changes := equinix_schema.GetResourceDataChangedKeys([]string{ networkDeviceLinkSchemaNames["Name"], networkDeviceLinkSchemaNames["Subnet"], networkDeviceLinkSchemaNames["Devices"], networkDeviceLinkSchemaNames["Links"], }, d) @@ -518,3 +519,14 @@ func networkDeviceLinkConnectionKey(v interface{}) string { func networkDeviceLinkConnectionHash(v interface{}) int { return hashcode.String(networkDeviceLinkConnectionKey(v)) } + +func schemaSetToMap(set *schema.Set) map[int]interface{} { + transformed := make(map[int]interface{}) + if set != nil { + list := set.List() + for i := range list { + transformed[set.F(list[i])] = list[i] + } + } + return transformed +} diff --git a/internal/schema/resourcedata.go b/internal/schema/resourcedata.go new file mode 100644 index 000000000..e4490bb63 --- /dev/null +++ b/internal/schema/resourcedata.go @@ -0,0 +1,46 @@ +package schema + +import "reflect" + +// resourceDataProvider provies interface to schema.ResourceData +// for convenient mocking purposes +type resourceDataProvider interface { + Get(key string) interface{} + GetOk(key string) (interface{}, bool) + HasChange(key string) bool + GetChange(key string) (interface{}, interface{}) +} + +func getMapChangedKeys(keys []string, old, new map[string]interface{}) map[string]interface{} { + changed := make(map[string]interface{}) + for _, key := range keys { + if !reflect.DeepEqual(old[key], new[key]) { + changed[key] = new[key] + } + } + return changed +} + +func GetResourceDataChangedKeys(keys []string, d resourceDataProvider) map[string]interface{} { + changed := make(map[string]interface{}) + for _, key := range keys { + if v := d.Get(key); v != nil && d.HasChange(key) { + changed[key] = v + } + } + return changed +} + +func GetResourceDataListElementChanges(keys []string, listKeyName string, listIndex int, d resourceDataProvider) map[string]interface{} { + changed := make(map[string]interface{}) + if !d.HasChange(listKeyName) { + return changed + } + old, new := d.GetChange(listKeyName) + oldList := old.([]interface{}) + newList := new.([]interface{}) + if len(oldList) < listIndex || len(newList) < listIndex { + return changed + } + return getMapChangedKeys(keys, oldList[listIndex].(map[string]interface{}), newList[listIndex].(map[string]interface{})) +} diff --git a/internal/schema/resourcedata_test.go b/internal/schema/resourcedata_test.go new file mode 100644 index 000000000..115caf29a --- /dev/null +++ b/internal/schema/resourcedata_test.go @@ -0,0 +1,106 @@ +package schema + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" +) + +type mockedResourceDataProvider struct { + actual map[string]interface{} + old map[string]interface{} +} + +func (r mockedResourceDataProvider) Get(key string) interface{} { + return r.actual[key] +} + +func (r mockedResourceDataProvider) GetOk(key string) (interface{}, bool) { + v, ok := r.actual[key] + return v, ok +} + +func (r mockedResourceDataProvider) HasChange(key string) bool { + return !reflect.DeepEqual(r.old[key], r.actual[key]) +} + +func (r mockedResourceDataProvider) GetChange(key string) (interface{}, interface{}) { + return r.old[key], r.actual[key] +} + +func TestProvider_resourceDataChangedKeys(t *testing.T) { + // given + keys := []string{"key", "keyTwo", "keyThree"} + rd := mockedResourceDataProvider{ + actual: map[string]interface{}{ + "key": "value", + "keyTwo": "newValueTwo", + }, + old: map[string]interface{}{ + "key": "value", + "keyTwo": "valueTwo", + }, + } + expected := map[string]interface{}{ + "keyTwo": "newValueTwo", + } + // when + result := GetResourceDataChangedKeys(keys, rd) + // then + assert.Equal(t, expected, result, "Function returns valid key changes") +} + +func TestProvider_resourceDataListElementChanges(t *testing.T) { + // given + keys := []string{"key", "keyTwo", "keyThree"} + listKeyName := "myList" + rd := mockedResourceDataProvider{ + old: map[string]interface{}{ + listKeyName: []interface{}{ + map[string]interface{}{ + "key": "value", + "keyTwo": "valueTwo", + "keyThree": 50, + }, + }, + }, + actual: map[string]interface{}{ + listKeyName: []interface{}{ + map[string]interface{}{ + "key": "value", + "keyTwo": "newValueTwo", + "keyThree": 100, + }, + }, + }, + } + expected := map[string]interface{}{ + "keyTwo": "newValueTwo", + "keyThree": 100, + } + // when + result := GetResourceDataListElementChanges(keys, listKeyName, 0, rd) + // then + assert.Equal(t, expected, result, "Function returns valid key changes") +} + +func TestProvider_mapChanges(t *testing.T) { + // given + keys := []string{"key", "keyTwo", "keyThree"} + old := map[string]interface{}{ + "key": "value", + "keyTwo": "valueTwo", + } + new := map[string]interface{}{ + "key": "newValue", + "keyTwo": "valueTwo", + } + expected := map[string]interface{}{ + "key": "newValue", + } + // when + result := getMapChangedKeys(keys, old, new) + // then + assert.Equal(t, expected, result, "Function returns valid key changes") +} diff --git a/internal/schema/set_map.go b/internal/schema/set_map.go index 426436f89..7a9ef1bf3 100644 --- a/internal/schema/set_map.go +++ b/internal/schema/set_map.go @@ -1,4 +1,4 @@ -package utils +package schema import ( "sort"