From f20a216fbfa60d0710ee0ccf4eaae70ab2333e90 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Thu, 26 May 2022 11:08:20 -0500 Subject: [PATCH 1/4] add value_from --- tfsdk/value_from.go | 22 +++++++ tfsdk/value_from_test.go | 128 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 tfsdk/value_from.go create mode 100644 tfsdk/value_from_test.go diff --git a/tfsdk/value_from.go b/tfsdk/value_from.go new file mode 100644 index 000000000..d3cd9a98b --- /dev/null +++ b/tfsdk/value_from.go @@ -0,0 +1,22 @@ +package tfsdk + +import ( + "context" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/internal/reflect" + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +// ValueFrom populates the attr value passed as `val` with +// the contents of `target`, using the reflection rules +// defined for `Get` and `GetAttribute`. +func ValueFrom(ctx context.Context, typ attr.Type, val *attr.Value, target interface{}) diag.Diagnostics { + v, diags := reflect.FromValue(ctx, typ, target, tftypes.NewAttributePath()) + if diags.HasError() { + return diags + } + + *val = v + return diags +} diff --git a/tfsdk/value_from_test.go b/tfsdk/value_from_test.go new file mode 100644 index 000000000..73a9470f9 --- /dev/null +++ b/tfsdk/value_from_test.go @@ -0,0 +1,128 @@ +package tfsdk + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +type person struct { + Name types.String `tfsdk:"name"` + Age types.Int64 `tfsdk:"age"` + OptedIn types.Bool `tfsdk:"opted_in"` +} + +func TestValueFrom(t *testing.T) { + t.Parallel() + + personAttrTypes := map[string]attr.Type{ + "name": types.StringType, + "age": types.Int64Type, + "opted_in": types.BoolType, + } + + x := person{ + Name: types.String{Value: "x"}, + Age: types.Int64{Value: 30}, + OptedIn: types.Bool{Value: true}, + } + + y := person{ + Name: types.String{Value: "y"}, + Age: types.Int64{Value: 23}, + OptedIn: types.Bool{Value: false}, + } + + xObj := types.Object{ + AttrTypes: personAttrTypes, + Attrs: map[string]attr.Value{ + "name": types.String{Value: "x", Unknown: false, Null: false}, + "age": types.Int64{Value: 30, Unknown: false, Null: false}, + "opted_in": types.Bool{Value: true, Unknown: false, Null: false}, + }, + } + + yObj := types.Object{ + AttrTypes: personAttrTypes, + Attrs: map[string]attr.Value{ + "name": types.String{Value: "y", Unknown: false, Null: false}, + "age": types.Int64{Value: 23, Unknown: false, Null: false}, + "opted_in": types.Bool{Value: false, Unknown: false, Null: false}, + }, + } + + type testCase struct { + target attr.Value + val interface{} + expected attr.Value + expectedDiags diag.Diagnostics + } + + tests := map[string]testCase{ + "primitive": { + val: "hello", + target: types.String{}, + expected: types.String{Value: "hello", Unknown: false, Null: false}, + }, + "struct": { + val: x, + target: types.Object{ + AttrTypes: personAttrTypes, + }, + expected: xObj, + }, + "list": { + val: []person{x, y}, + target: types.List{ + ElemType: types.ObjectType{ + AttrTypes: personAttrTypes, + }, + }, + expected: types.List{ + ElemType: types.ObjectType{ + AttrTypes: personAttrTypes, + }, + Elems: []attr.Value{xObj, yObj}, + }, + }, + //"incompatible-type": { + // val: 0, + // target: types.String{}, + // expectedDiags: diag.Diagnostics{ + // diag.WithPath( + // tftypes.NewAttributePath(), + // reflect.DiagIntoIncompatibleType{ + // Val: tftypes.NewValue(tftypes.String, ""), + // TargetType: goreflect.TypeOf(int64(0)), + // Err: fmt.Errorf("unexpected error was encountered trying to convert from value"), + // }, + // ), + // }, + //}, + } + + for name, tc := range tests { + name, tc := name, tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + diags := ValueFrom(context.Background(), tc.target.Type(context.Background()), &tc.target, &tc.val) + + if diff := cmp.Diff(tc.expectedDiags, diags); diff != "" { + t.Fatalf("Unexpected diff in diagnostics (-wanted, +got): %s", diff) + } + + if diags.HasError() { + return + } + + if diff := cmp.Diff(tc.expected, tc.target); diff != "" { + t.Fatalf("Unexpected diff in results (-wanted, +got): %s", diff) + } + }) + } +} From 1600e23c456fb3130c3604edb4f631f24d8b01fe Mon Sep 17 00:00:00 2001 From: Ivan De Marino Date: Fri, 27 May 2022 17:00:15 +0100 Subject: [PATCH 2/4] Allowing `ValueFrom` to set the resulting converted value on any compatible pointer provided as `target` --- tfsdk/value_as.go | 6 +-- tfsdk/value_from.go | 15 +++---- tfsdk/value_from_test.go | 84 ++++++++++++++++++++++++++++------------ 3 files changed, 71 insertions(+), 34 deletions(-) diff --git a/tfsdk/value_as.go b/tfsdk/value_as.go index 2b60d0ac1..45ba22c65 100644 --- a/tfsdk/value_as.go +++ b/tfsdk/value_as.go @@ -9,9 +9,9 @@ import ( "github.com/hashicorp/terraform-plugin-framework/internal/reflect" ) -// ValueAs populates the Go value passed as `target` with -// the contents of `val`, using the reflection rules -// defined for `Get` and `GetAttribute`. +// ValueAs takes the attr.Value `val` and populates the Go value `target` with its content. +// +// This is achieved using reflection rules provided by the internal/reflect package. func ValueAs(ctx context.Context, val attr.Value, target interface{}) diag.Diagnostics { if reflect.IsGenericAttrValue(ctx, target) { *(target.(*attr.Value)) = val diff --git a/tfsdk/value_from.go b/tfsdk/value_from.go index d3cd9a98b..f0445ca0d 100644 --- a/tfsdk/value_from.go +++ b/tfsdk/value_from.go @@ -2,21 +2,22 @@ package tfsdk import ( "context" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/reflect" "github.com/hashicorp/terraform-plugin-go/tftypes" ) -// ValueFrom populates the attr value passed as `val` with -// the contents of `target`, using the reflection rules -// defined for `Get` and `GetAttribute`. -func ValueFrom(ctx context.Context, typ attr.Type, val *attr.Value, target interface{}) diag.Diagnostics { - v, diags := reflect.FromValue(ctx, typ, target, tftypes.NewAttributePath()) +// ValueFrom takes the Go value `val` and populates `target` with an attr.Value, +// based on the type definition provided in `targetType`. +// +// This is achieved using reflection rules provided by the internal/reflect package. +func ValueFrom(ctx context.Context, val interface{}, targetType attr.Type, target interface{}) diag.Diagnostics { + v, diags := reflect.FromValue(ctx, targetType, val, tftypes.NewAttributePath()) if diags.HasError() { return diags } - *val = v - return diags + return ValueAs(ctx, v, target) } diff --git a/tfsdk/value_from_test.go b/tfsdk/value_from_test.go index 73a9470f9..d78326a96 100644 --- a/tfsdk/value_from_test.go +++ b/tfsdk/value_from_test.go @@ -8,12 +8,14 @@ import ( "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-go/tftypes" ) type person struct { Name types.String `tfsdk:"name"` Age types.Int64 `tfsdk:"age"` OptedIn types.Bool `tfsdk:"opted_in"` + Address types.List `tfsdk:"address"` } func TestValueFrom(t *testing.T) { @@ -23,41 +25,76 @@ func TestValueFrom(t *testing.T) { "name": types.StringType, "age": types.Int64Type, "opted_in": types.BoolType, + "address": types.ListType{ + ElemType: types.StringType, + }, } - x := person{ + mrX := person{ Name: types.String{Value: "x"}, Age: types.Int64{Value: 30}, OptedIn: types.Bool{Value: true}, + Address: types.List{ + ElemType: types.StringType, + Elems: []attr.Value{ + types.String{Value: "1"}, + types.String{Value: "Beckford Close"}, + types.String{Value: "Gotham"}, + }, + }, } - y := person{ + mrsY := person{ Name: types.String{Value: "y"}, Age: types.Int64{Value: 23}, OptedIn: types.Bool{Value: false}, + Address: types.List{ + ElemType: types.StringType, + Elems: []attr.Value{ + types.String{Value: "2"}, + types.String{Value: "Windmill Close"}, + types.String{Value: "Smallville"}, + }, + }, } - xObj := types.Object{ + expectedMrXObj := types.Object{ AttrTypes: personAttrTypes, Attrs: map[string]attr.Value{ "name": types.String{Value: "x", Unknown: false, Null: false}, "age": types.Int64{Value: 30, Unknown: false, Null: false}, "opted_in": types.Bool{Value: true, Unknown: false, Null: false}, + "address": types.List{ + ElemType: types.StringType, + Elems: []attr.Value{ + types.String{Value: "1"}, + types.String{Value: "Beckford Close"}, + types.String{Value: "Gotham"}, + }, + }, }, } - yObj := types.Object{ + expectedMrsYObj := types.Object{ AttrTypes: personAttrTypes, Attrs: map[string]attr.Value{ "name": types.String{Value: "y", Unknown: false, Null: false}, "age": types.Int64{Value: 23, Unknown: false, Null: false}, "opted_in": types.Bool{Value: false, Unknown: false, Null: false}, + "address": types.List{ + ElemType: types.StringType, + Elems: []attr.Value{ + types.String{Value: "2"}, + types.String{Value: "Windmill Close"}, + types.String{Value: "Smallville"}, + }, + }, }, } type testCase struct { - target attr.Value val interface{} + target attr.Value expected attr.Value expectedDiags diag.Diagnostics } @@ -69,14 +106,14 @@ func TestValueFrom(t *testing.T) { expected: types.String{Value: "hello", Unknown: false, Null: false}, }, "struct": { - val: x, + val: mrX, target: types.Object{ AttrTypes: personAttrTypes, }, - expected: xObj, + expected: expectedMrXObj, }, "list": { - val: []person{x, y}, + val: []person{mrX, mrsY}, target: types.List{ ElemType: types.ObjectType{ AttrTypes: personAttrTypes, @@ -86,23 +123,22 @@ func TestValueFrom(t *testing.T) { ElemType: types.ObjectType{ AttrTypes: personAttrTypes, }, - Elems: []attr.Value{xObj, yObj}, + Elems: []attr.Value{expectedMrXObj, expectedMrsYObj}, + }, + }, + "incompatible-type": { + val: 0, + target: types.String{}, + expectedDiags: diag.Diagnostics{ + diag.WithPath( + tftypes.NewAttributePath(), + diag.NewErrorDiagnostic( + "Value Conversion Error", + "An unexpected error was encountered trying to convert the Terraform value. This is always an error in the provider. Please report the following to the provider developer:\n\ncan't unmarshal tftypes.Number into *string, expected string", + ), + ), }, }, - //"incompatible-type": { - // val: 0, - // target: types.String{}, - // expectedDiags: diag.Diagnostics{ - // diag.WithPath( - // tftypes.NewAttributePath(), - // reflect.DiagIntoIncompatibleType{ - // Val: tftypes.NewValue(tftypes.String, ""), - // TargetType: goreflect.TypeOf(int64(0)), - // Err: fmt.Errorf("unexpected error was encountered trying to convert from value"), - // }, - // ), - // }, - //}, } for name, tc := range tests { @@ -110,7 +146,7 @@ func TestValueFrom(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - diags := ValueFrom(context.Background(), tc.target.Type(context.Background()), &tc.target, &tc.val) + diags := ValueFrom(context.Background(), tc.val, tc.target.Type(context.Background()), &tc.target) if diff := cmp.Diff(tc.expectedDiags, diags); diff != "" { t.Fatalf("Unexpected diff in diagnostics (-wanted, +got): %s", diff) From e7024fdff9f77257e73d8ca3b3e2c9ec78c6c51b Mon Sep 17 00:00:00 2001 From: Ivan De Marino Date: Fri, 27 May 2022 17:03:44 +0100 Subject: [PATCH 3/4] Added CHANGELOG entry --- .changelog/350.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/350.txt diff --git a/.changelog/350.txt b/.changelog/350.txt new file mode 100644 index 000000000..5101d5169 --- /dev/null +++ b/.changelog/350.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +tfsdk: New function `ValueFrom` that takes a Go value and populates a compatible `attr.Value`, given a descriptive `attr.Type`. +``` From 2c2622ea0653e5546d44bf4c4c8f50f0e031f77c Mon Sep 17 00:00:00 2001 From: Ivan De Marino Date: Mon, 30 May 2022 09:40:49 +0100 Subject: [PATCH 4/4] Adding test case to unmarshal 'ValueFrom' a Map --- tfsdk/value_from_test.go | 64 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 4 deletions(-) diff --git a/tfsdk/value_from_test.go b/tfsdk/value_from_test.go index d78326a96..e52181f12 100644 --- a/tfsdk/value_from_test.go +++ b/tfsdk/value_from_test.go @@ -12,10 +12,11 @@ import ( ) type person struct { - Name types.String `tfsdk:"name"` - Age types.Int64 `tfsdk:"age"` - OptedIn types.Bool `tfsdk:"opted_in"` - Address types.List `tfsdk:"address"` + Name types.String `tfsdk:"name"` + Age types.Int64 `tfsdk:"age"` + OptedIn types.Bool `tfsdk:"opted_in"` + Address types.List `tfsdk:"address"` + FullName types.Map `tfsdk:"full_name"` } func TestValueFrom(t *testing.T) { @@ -28,6 +29,9 @@ func TestValueFrom(t *testing.T) { "address": types.ListType{ ElemType: types.StringType, }, + "full_name": types.MapType{ + ElemType: types.StringType, + }, } mrX := person{ @@ -42,6 +46,14 @@ func TestValueFrom(t *testing.T) { types.String{Value: "Gotham"}, }, }, + FullName: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "first": types.String{Value: "x"}, + "middle": types.String{Value: "b"}, + "last": types.String{Value: "c"}, + }, + }, } mrsY := person{ @@ -56,6 +68,14 @@ func TestValueFrom(t *testing.T) { types.String{Value: "Smallville"}, }, }, + FullName: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "first": types.String{Value: "y"}, + "middle": types.String{Value: "e"}, + "last": types.String{Value: "f"}, + }, + }, } expectedMrXObj := types.Object{ @@ -72,6 +92,14 @@ func TestValueFrom(t *testing.T) { types.String{Value: "Gotham"}, }, }, + "full_name": types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "first": types.String{Value: "x"}, + "middle": types.String{Value: "b"}, + "last": types.String{Value: "c"}, + }, + }, }, } @@ -89,6 +117,14 @@ func TestValueFrom(t *testing.T) { types.String{Value: "Smallville"}, }, }, + "full_name": types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "first": types.String{Value: "y"}, + "middle": types.String{Value: "e"}, + "last": types.String{Value: "f"}, + }, + }, }, } @@ -126,6 +162,26 @@ func TestValueFrom(t *testing.T) { Elems: []attr.Value{expectedMrXObj, expectedMrsYObj}, }, }, + "map": { + val: map[string]person{ + "x": mrX, + "y": mrsY, + }, + target: types.Map{ + ElemType: types.ObjectType{ + AttrTypes: personAttrTypes, + }, + }, + expected: types.Map{ + ElemType: types.ObjectType{ + AttrTypes: personAttrTypes, + }, + Elems: map[string]attr.Value{ + "x": expectedMrXObj, + "y": expectedMrsYObj, + }, + }, + }, "incompatible-type": { val: 0, target: types.String{},