From caf717a1455a1bbf3b4c2988007cb2dfab8de9d9 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Tue, 10 Dec 2024 13:02:33 -0500 Subject: [PATCH 1/4] whitespace --- apstra/custom_types/ipv46_address_type.go | 3 --- apstra/custom_types/ipv46_address_value.go | 1 - 2 files changed, 4 deletions(-) diff --git a/apstra/custom_types/ipv46_address_type.go b/apstra/custom_types/ipv46_address_type.go index 9ea1581e..944967fe 100644 --- a/apstra/custom_types/ipv46_address_type.go +++ b/apstra/custom_types/ipv46_address_type.go @@ -50,19 +50,16 @@ func (t IPv46AddressType) ValueFromString(_ context.Context, in basetypes.String // for the provider to consume the data with. func (t IPv46AddressType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { attrValue, err := t.StringType.ValueFromTerraform(ctx, in) - if err != nil { return nil, err } stringValue, ok := attrValue.(basetypes.StringValue) - if !ok { return nil, fmt.Errorf("unexpected value type of %T", attrValue) } stringValuable, diags := t.ValueFromString(ctx, stringValue) - if diags.HasError() { return nil, fmt.Errorf("unexpected error converting StringValue to StringValuable: %v", diags) } diff --git a/apstra/custom_types/ipv46_address_value.go b/apstra/custom_types/ipv46_address_value.go index 1f69713d..7172f3a9 100644 --- a/apstra/custom_types/ipv46_address_value.go +++ b/apstra/custom_types/ipv46_address_value.go @@ -71,7 +71,6 @@ func (v IPv46Address) Type(_ context.Context) attr.Type { func (v IPv46Address) Equal(o attr.Value) bool { other, ok := o.(IPv46Address) - if !ok { return false } From 19ee77e13cc6ce55fb82c3b7e556ef3e36fbd274 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Tue, 10 Dec 2024 13:05:31 -0500 Subject: [PATCH 2/4] introduce --- .../string_with_alt_values_type.go | 68 +++++++++++++++ .../string_with_alt_values_type_test.go | 56 ++++++++++++ .../string_with_alt_values_value.go | 87 +++++++++++++++++++ .../string_with_alt_values_value_test.go | 50 +++++++++++ 4 files changed, 261 insertions(+) create mode 100644 apstra/custom_types/string_with_alt_values_type.go create mode 100644 apstra/custom_types/string_with_alt_values_type_test.go create mode 100644 apstra/custom_types/string_with_alt_values_value.go create mode 100644 apstra/custom_types/string_with_alt_values_value_test.go diff --git a/apstra/custom_types/string_with_alt_values_type.go b/apstra/custom_types/string_with_alt_values_type.go new file mode 100644 index 00000000..62008015 --- /dev/null +++ b/apstra/custom_types/string_with_alt_values_type.go @@ -0,0 +1,68 @@ +package customtypes + +import ( + "context" + "fmt" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +var ( + _ basetypes.StringTypable = (*StringWithAltValuesType)(nil) + _ attr.Type = (*StringWithAltValuesType)(nil) +) + +type StringWithAltValuesType struct { + basetypes.StringType +} + +// String returns a human readable string of the type name. +func (t StringWithAltValuesType) String() string { + return "customtypes.StringWithAltValues" +} + +// ValueType returns the Value type. +func (t StringWithAltValuesType) ValueType(_ context.Context) attr.Value { + return StringWithAltValues{} +} + +// Equal returns true if the given type is equivalent. +func (t StringWithAltValuesType) Equal(o attr.Type) bool { + other, ok := o.(StringWithAltValuesType) + + if !ok { + return false + } + + return t.StringType.Equal(other.StringType) +} + +// ValueFromString returns a StringValuable type given a StringValue. +func (t StringWithAltValuesType) ValueFromString(_ context.Context, in basetypes.StringValue) (basetypes.StringValuable, diag.Diagnostics) { + return StringWithAltValues{ + StringValue: in, + }, nil +} + +// ValueFromTerraform returns a Value given a tftypes.Value. This is meant to convert the tftypes.Value into a more convenient Go type +// for the provider to consume the data with. +func (t StringWithAltValuesType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { + attrValue, err := t.StringType.ValueFromTerraform(ctx, in) + if err != nil { + return nil, err + } + + stringValue, ok := attrValue.(basetypes.StringValue) + if !ok { + return nil, fmt.Errorf("unexpected value type of %T", attrValue) + } + + stringValuable, diags := t.ValueFromString(ctx, stringValue) + if diags.HasError() { + return nil, fmt.Errorf("unexpected error converting StringValue to StringValuable: %v", diags) + } + + return stringValuable, nil +} diff --git a/apstra/custom_types/string_with_alt_values_type_test.go b/apstra/custom_types/string_with_alt_values_type_test.go new file mode 100644 index 00000000..271d73b6 --- /dev/null +++ b/apstra/custom_types/string_with_alt_values_type_test.go @@ -0,0 +1,56 @@ +package customtypes_test + +import ( + "context" + customtypes "github.com/Juniper/terraform-provider-apstra/apstra/custom_types" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/stretchr/testify/require" + "testing" +) + +func TestStringWithAltValuesType_ValueFromTerraform(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + in tftypes.Value + expectation attr.Value + expectedErr string + }{ + "true": { + in: tftypes.NewValue(tftypes.String, "foo"), + expectation: customtypes.NewStringWithAltValuesValue("foo"), + }, + "unknown": { + in: tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + expectation: customtypes.NewStringWithAltValuesUnknown(), + }, + "null": { + in: tftypes.NewValue(tftypes.String, nil), + expectation: customtypes.NewStringWithAltValuesNull(), + }, + "wrongType": { + in: tftypes.NewValue(tftypes.Number, 123), + expectedErr: "can't unmarshal tftypes.Number into *string, expected string", + }, + } + + for tName, tCase := range testCases { + t.Run(tName, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + + got, err := customtypes.StringWithAltValuesType{}.ValueFromTerraform(ctx, tCase.in) + if tCase.expectedErr == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Equal(t, tCase.expectedErr, err.Error()) + return + } + + require.Truef(t, got.Equal(tCase.expectation), "values not equal %s, %s", tCase.expectation, got) + }) + } + +} diff --git a/apstra/custom_types/string_with_alt_values_value.go b/apstra/custom_types/string_with_alt_values_value.go new file mode 100644 index 00000000..9e6e0117 --- /dev/null +++ b/apstra/custom_types/string_with_alt_values_value.go @@ -0,0 +1,87 @@ +package customtypes + +import ( + "context" + "fmt" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" +) + +var ( + _ basetypes.StringValuable = (*StringWithAltValues)(nil) + _ basetypes.StringValuableWithSemanticEquals = (*StringWithAltValues)(nil) +) + +type StringWithAltValues struct { + basetypes.StringValue + altValues []attr.Value +} + +func (v StringWithAltValues) Type(_ context.Context) attr.Type { + return StringWithAltValuesType{} +} + +func (v StringWithAltValues) Equal(o attr.Value) bool { + other, ok := o.(StringWithAltValues) + if !ok { + return false + } + + return v.StringValue.Equal(other.StringValue) +} + +func (v StringWithAltValues) StringSemanticEquals(_ context.Context, newValuable basetypes.StringValuable) (bool, diag.Diagnostics) { + var diags diag.Diagnostics + + newValue, ok := newValuable.(StringWithAltValues) + if !ok { + diags.AddError( + "Semantic Equality Check Error", + "An unexpected value type was received while performing semantic equality checks. "+ + "Please report this to the provider developers.\n\n"+ + "Expected Value Type: "+fmt.Sprintf("%T", v)+"\n"+ + "Got Value Type: "+fmt.Sprintf("%T", newValuable), + ) + + return false, diags + } + + // check new value against our "main" value + if v.Equal(newValue) { + return true, diags + } + + // check new value against our "alt" values + for _, a := range v.altValues { + if a.Equal(newValue) { + return true, diags + } + } + + return false, diags +} + +func NewStringWithAltValuesNull() StringWithAltValues { + return StringWithAltValues{ + StringValue: basetypes.NewStringNull(), + } +} + +func NewStringWithAltValuesUnknown() StringWithAltValues { + return StringWithAltValues{ + StringValue: basetypes.NewStringUnknown(), + } +} + +func NewStringWithAltValuesValue(value string, alt ...string) StringWithAltValues { + altValues := make([]attr.Value, len(alt)) + for i, a := range alt { + altValues[i] = StringWithAltValues{StringValue: basetypes.NewStringValue(a)} + } + + return StringWithAltValues{ + StringValue: basetypes.NewStringValue(value), + altValues: altValues, + } +} diff --git a/apstra/custom_types/string_with_alt_values_value_test.go b/apstra/custom_types/string_with_alt_values_value_test.go new file mode 100644 index 00000000..af0dfdfd --- /dev/null +++ b/apstra/custom_types/string_with_alt_values_value_test.go @@ -0,0 +1,50 @@ +package customtypes_test + +import ( + "context" + customtypes "github.com/Juniper/terraform-provider-apstra/apstra/custom_types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + "github.com/stretchr/testify/require" + "testing" +) + +func TestStringWithAltValues_StringSemanticEquals(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + currentValue customtypes.StringWithAltValues + givenValue basetypes.StringValuable + expectedMatch bool + }{ + "equal - no alt values": { + currentValue: customtypes.NewStringWithAltValuesValue("foo"), + givenValue: customtypes.NewStringWithAltValuesValue("foo"), + expectedMatch: true, + }, + "equal - with alt values": { + currentValue: customtypes.NewStringWithAltValuesValue("foo", "bar", "baz"), + givenValue: customtypes.NewStringWithAltValuesValue("foo"), + expectedMatch: true, + }, + "semantically equal": { + currentValue: customtypes.NewStringWithAltValuesValue("foo", "bar", "baz", "bang"), + givenValue: customtypes.NewStringWithAltValuesValue("baz"), + expectedMatch: true, + }, + "not equal": { + currentValue: customtypes.NewStringWithAltValuesValue("foo", "bar", "baz", "bang"), + givenValue: customtypes.NewStringWithAltValuesValue("FOO"), + expectedMatch: false, + }, + } + + for tName, tCase := range testCases { + t.Run(tName, func(t *testing.T) { + t.Parallel() + + match, diags := tCase.currentValue.StringSemanticEquals(context.Background(), tCase.givenValue) + require.Equalf(t, tCase.expectedMatch, match, "Expected StringSemanticEquals to return: %t, but got: %t", tCase.expectedMatch, match) + require.Nil(t, diags) + }) + } +} From 265d56ebad6b83d0c94cef8b7513344e34d4625b Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Tue, 10 Dec 2024 13:42:31 -0500 Subject: [PATCH 3/4] make semantic equality symmetrical --- apstra/custom_types/string_with_alt_values_value.go | 10 ++++++++++ .../custom_types/string_with_alt_values_value_test.go | 7 ++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/apstra/custom_types/string_with_alt_values_value.go b/apstra/custom_types/string_with_alt_values_value.go index 9e6e0117..fab830de 100644 --- a/apstra/custom_types/string_with_alt_values_value.go +++ b/apstra/custom_types/string_with_alt_values_value.go @@ -31,6 +31,9 @@ func (v StringWithAltValues) Equal(o attr.Value) bool { return v.StringValue.Equal(other.StringValue) } +// StringSemanticEquals implements the semantic equality check. According to this +// (https://discuss.hashicorp.com/t/can-semantic-equality-check-in-custom-types-be-asymmetrical/60644/2?u=hqnvylrx) +// semantic equality checks on custom types are always implementeed as oldValue.SemanticEquals(ctx, newValue) func (v StringWithAltValues) StringSemanticEquals(_ context.Context, newValuable basetypes.StringValuable) (bool, diag.Diagnostics) { var diags diag.Diagnostics @@ -59,6 +62,13 @@ func (v StringWithAltValues) StringSemanticEquals(_ context.Context, newValuable } } + // check old value against new "alt" values + for _, a := range newValue.altValues { + if a.Equal(v) { + return true, diags + } + } + return false, diags } diff --git a/apstra/custom_types/string_with_alt_values_value_test.go b/apstra/custom_types/string_with_alt_values_value_test.go index af0dfdfd..72b1c939 100644 --- a/apstra/custom_types/string_with_alt_values_value_test.go +++ b/apstra/custom_types/string_with_alt_values_value_test.go @@ -26,11 +26,16 @@ func TestStringWithAltValues_StringSemanticEquals(t *testing.T) { givenValue: customtypes.NewStringWithAltValuesValue("foo"), expectedMatch: true, }, - "semantically equal": { + "semantically equal - given matches an alt value": { currentValue: customtypes.NewStringWithAltValuesValue("foo", "bar", "baz", "bang"), givenValue: customtypes.NewStringWithAltValuesValue("baz"), expectedMatch: true, }, + "semantically equal - current matches an alt value": { + currentValue: customtypes.NewStringWithAltValuesValue("baz"), + givenValue: customtypes.NewStringWithAltValuesValue("foo", "bar", "baz", "bang"), + expectedMatch: true, + }, "not equal": { currentValue: customtypes.NewStringWithAltValuesValue("foo", "bar", "baz", "bang"), givenValue: customtypes.NewStringWithAltValuesValue("FOO"), From fac9ee98ad7b2ef72e8ab84e1f37a352ba8bd8f6 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Tue, 10 Dec 2024 13:46:32 -0500 Subject: [PATCH 4/4] gofumpt --- apstra/custom_types/ipv46_address_type.go | 1 + apstra/custom_types/string_with_alt_values_type.go | 1 + apstra/custom_types/string_with_alt_values_type_test.go | 4 ++-- apstra/custom_types/string_with_alt_values_value.go | 1 + apstra/custom_types/string_with_alt_values_value_test.go | 3 ++- 5 files changed, 7 insertions(+), 3 deletions(-) diff --git a/apstra/custom_types/ipv46_address_type.go b/apstra/custom_types/ipv46_address_type.go index 944967fe..7b80c028 100644 --- a/apstra/custom_types/ipv46_address_type.go +++ b/apstra/custom_types/ipv46_address_type.go @@ -3,6 +3,7 @@ package customtypes import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" diff --git a/apstra/custom_types/string_with_alt_values_type.go b/apstra/custom_types/string_with_alt_values_type.go index 62008015..9d354270 100644 --- a/apstra/custom_types/string_with_alt_values_type.go +++ b/apstra/custom_types/string_with_alt_values_type.go @@ -3,6 +3,7 @@ package customtypes import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" diff --git a/apstra/custom_types/string_with_alt_values_type_test.go b/apstra/custom_types/string_with_alt_values_type_test.go index 271d73b6..b7f9e5bf 100644 --- a/apstra/custom_types/string_with_alt_values_type_test.go +++ b/apstra/custom_types/string_with_alt_values_type_test.go @@ -2,11 +2,12 @@ package customtypes_test import ( "context" + "testing" + customtypes "github.com/Juniper/terraform-provider-apstra/apstra/custom_types" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/stretchr/testify/require" - "testing" ) func TestStringWithAltValuesType_ValueFromTerraform(t *testing.T) { @@ -52,5 +53,4 @@ func TestStringWithAltValuesType_ValueFromTerraform(t *testing.T) { require.Truef(t, got.Equal(tCase.expectation), "values not equal %s, %s", tCase.expectation, got) }) } - } diff --git a/apstra/custom_types/string_with_alt_values_value.go b/apstra/custom_types/string_with_alt_values_value.go index fab830de..d985e0d9 100644 --- a/apstra/custom_types/string_with_alt_values_value.go +++ b/apstra/custom_types/string_with_alt_values_value.go @@ -3,6 +3,7 @@ package customtypes import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" diff --git a/apstra/custom_types/string_with_alt_values_value_test.go b/apstra/custom_types/string_with_alt_values_value_test.go index 72b1c939..006509f0 100644 --- a/apstra/custom_types/string_with_alt_values_value_test.go +++ b/apstra/custom_types/string_with_alt_values_value_test.go @@ -2,10 +2,11 @@ package customtypes_test import ( "context" + "testing" + customtypes "github.com/Juniper/terraform-provider-apstra/apstra/custom_types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/stretchr/testify/require" - "testing" ) func TestStringWithAltValues_StringSemanticEquals(t *testing.T) {