Skip to content

Commit

Permalink
tfsdk: Additional preparations for multiple schema implementations (#440
Browse files Browse the repository at this point in the history
)

Reference: #365

This is a followup to the initial schema logic migration that tries to further prepare the Go module for multiple schema implementations.

- Removes unexported schema-based functions/methods from the `tfsdk` package (`pathMatches` will be handled as part of schema data internalization)
- Removes `tftypes.Type` handling from schemas as `(attr.Type).TerraformType()` already does this
- Tries to standardize on `Type()` for fetching framework type (`attr.Type`) information from schema types (`Attribute` will get fixed once `tfsdk.Attribute` is removed)
- Tries to standardize `Schema` method signatures for framework types/diagnostics and terraform-plugin-go types/errors
- Prepares `NestedAttributes` implementations for unit testing across multiple attribute implementations
  • Loading branch information
bflad committed Aug 8, 2022
1 parent e6bc60e commit 0ba16ff
Show file tree
Hide file tree
Showing 50 changed files with 2,057 additions and 491 deletions.
15 changes: 15 additions & 0 deletions .changelog/440.txt
@@ -0,0 +1,15 @@
```release-note:note
tfsdk: The `Schema` type `AttributeAtPath()` method signature will be updated from a `*tftypes.AttributePath` parameter to `path.Path` in the next release. Switch to the `AttributeAtTerraformPath()` method if `*tftypes.AttributePath` handling is still necessary.
```

```release-note:note
tfsdk: The `Schema` type `AttributeTypeAtPath()` method has been deprecated for the `TypeAtPath()` and `TypeAtTerraformPath()` methods.
```

```release-note:note
tfsdk: The `Schema` type `AttributeType()` method has been deprecated in preference of the `Type()` method.
```

```release-note:note
tfsdk: The `Schema` type `TerraformType()` method has been deprecated in preference of calling `Type().TerraformType()`.
```
2 changes: 1 addition & 1 deletion internal/fromproto5/config.go
Expand Up @@ -32,7 +32,7 @@ func Config(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValue, sch
return nil, diags
}

proto5Value, err := proto5DynamicValue.Unmarshal(schema.TerraformType(ctx))
proto5Value, err := proto5DynamicValue.Unmarshal(schema.Type().TerraformType(ctx))

if err != nil {
diags.AddError(
Expand Down
2 changes: 1 addition & 1 deletion internal/fromproto5/importresourcestate.go
Expand Up @@ -37,7 +37,7 @@ func ImportResourceStateRequest(ctx context.Context, proto5 *tfprotov5.ImportRes

fw := &fwserver.ImportResourceStateRequest{
EmptyState: tfsdk.State{
Raw: tftypes.NewValue(resourceSchema.TerraformType(ctx), nil),
Raw: tftypes.NewValue(resourceSchema.Type().TerraformType(ctx), nil),
Schema: tfsdkSchema(resourceSchema),
},
ID: proto5.ID,
Expand Down
2 changes: 1 addition & 1 deletion internal/fromproto5/importresourcestate_test.go
Expand Up @@ -29,7 +29,7 @@ func TestImportResourceStateRequest(t *testing.T) {
}

testFwEmptyState := tfsdk.State{
Raw: tftypes.NewValue(testFwSchema.TerraformType(context.Background()), nil),
Raw: tftypes.NewValue(testFwSchema.Type().TerraformType(context.Background()), nil),
Schema: *testFwSchema,
}

Expand Down
2 changes: 1 addition & 1 deletion internal/fromproto5/plan.go
Expand Up @@ -32,7 +32,7 @@ func Plan(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValue, schem
return nil, diags
}

proto5Value, err := proto5DynamicValue.Unmarshal(schema.TerraformType(ctx))
proto5Value, err := proto5DynamicValue.Unmarshal(schema.Type().TerraformType(ctx))

if err != nil {
diags.AddError(
Expand Down
4 changes: 2 additions & 2 deletions internal/fromproto5/providermeta.go
Expand Up @@ -24,15 +24,15 @@ func ProviderMeta(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValu
var diags diag.Diagnostics

fw := &tfsdk.Config{
Raw: tftypes.NewValue(schema.TerraformType(ctx), nil),
Raw: tftypes.NewValue(schema.Type().TerraformType(ctx), nil),
Schema: tfsdkSchema(schema),
}

if proto5DynamicValue == nil {
return fw, nil
}

proto5Value, err := proto5DynamicValue.Unmarshal(schema.TerraformType(ctx))
proto5Value, err := proto5DynamicValue.Unmarshal(schema.Type().TerraformType(ctx))

if err != nil {
diags.AddError(
Expand Down
2 changes: 1 addition & 1 deletion internal/fromproto5/state.go
Expand Up @@ -32,7 +32,7 @@ func State(ctx context.Context, proto5DynamicValue *tfprotov5.DynamicValue, sche
return nil, diags
}

proto5Value, err := proto5DynamicValue.Unmarshal(schema.TerraformType(ctx))
proto5Value, err := proto5DynamicValue.Unmarshal(schema.Type().TerraformType(ctx))

if err != nil {
diags.AddError(
Expand Down
2 changes: 1 addition & 1 deletion internal/fromproto6/config.go
Expand Up @@ -32,7 +32,7 @@ func Config(ctx context.Context, proto6DynamicValue *tfprotov6.DynamicValue, sch
return nil, diags
}

proto6Value, err := proto6DynamicValue.Unmarshal(schema.TerraformType(ctx))
proto6Value, err := proto6DynamicValue.Unmarshal(schema.Type().TerraformType(ctx))

if err != nil {
diags.AddError(
Expand Down
2 changes: 1 addition & 1 deletion internal/fromproto6/importresourcestate.go
Expand Up @@ -37,7 +37,7 @@ func ImportResourceStateRequest(ctx context.Context, proto6 *tfprotov6.ImportRes

fw := &fwserver.ImportResourceStateRequest{
EmptyState: tfsdk.State{
Raw: tftypes.NewValue(resourceSchema.TerraformType(ctx), nil),
Raw: tftypes.NewValue(resourceSchema.Type().TerraformType(ctx), nil),
Schema: tfsdkSchema(resourceSchema),
},
ID: proto6.ID,
Expand Down
2 changes: 1 addition & 1 deletion internal/fromproto6/importresourcestate_test.go
Expand Up @@ -29,7 +29,7 @@ func TestImportResourceStateRequest(t *testing.T) {
}

testFwEmptyState := tfsdk.State{
Raw: tftypes.NewValue(testFwSchema.TerraformType(context.Background()), nil),
Raw: tftypes.NewValue(testFwSchema.Type().TerraformType(context.Background()), nil),
Schema: *testFwSchema,
}

Expand Down
2 changes: 1 addition & 1 deletion internal/fromproto6/plan.go
Expand Up @@ -32,7 +32,7 @@ func Plan(ctx context.Context, proto6DynamicValue *tfprotov6.DynamicValue, schem
return nil, diags
}

proto6Value, err := proto6DynamicValue.Unmarshal(schema.TerraformType(ctx))
proto6Value, err := proto6DynamicValue.Unmarshal(schema.Type().TerraformType(ctx))

if err != nil {
diags.AddError(
Expand Down
4 changes: 2 additions & 2 deletions internal/fromproto6/providermeta.go
Expand Up @@ -24,15 +24,15 @@ func ProviderMeta(ctx context.Context, proto6DynamicValue *tfprotov6.DynamicValu
var diags diag.Diagnostics

fw := &tfsdk.Config{
Raw: tftypes.NewValue(schema.TerraformType(ctx), nil),
Raw: tftypes.NewValue(schema.Type().TerraformType(ctx), nil),
Schema: tfsdkSchema(schema),
}

if proto6DynamicValue == nil {
return fw, nil
}

proto6Value, err := proto6DynamicValue.Unmarshal(schema.TerraformType(ctx))
proto6Value, err := proto6DynamicValue.Unmarshal(schema.Type().TerraformType(ctx))

if err != nil {
diags.AddError(
Expand Down
2 changes: 1 addition & 1 deletion internal/fromproto6/state.go
Expand Up @@ -32,7 +32,7 @@ func State(ctx context.Context, proto6DynamicValue *tfprotov6.DynamicValue, sche
return nil, diags
}

proto6Value, err := proto6DynamicValue.Unmarshal(schema.TerraformType(ctx))
proto6Value, err := proto6DynamicValue.Unmarshal(schema.Type().TerraformType(ctx))

if err != nil {
diags.AddError(
Expand Down
@@ -1,30 +1,23 @@
package tfsdk
package fromtftypes

import (
"context"
"fmt"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromtftypes"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

// attributePath returns the path.Path equivalent of a *tftypes.AttributePath.
//
// TODO: This function should be exported as internal/fromtftypes.AttributePath
// except that doing so would currently introduce an import cycle due to the
// tfsdk.Schema parameter here and Config/Plan/State.PathMatches needing to
// call this function until the schema data is migrated to attr.Value.
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/172
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/365
func attributePath(ctx context.Context, tfType *tftypes.AttributePath, schema Schema) (path.Path, diag.Diagnostics) {
// AttributePath returns the path.Path equivalent of a *tftypes.AttributePath.
func AttributePath(ctx context.Context, tfType *tftypes.AttributePath, schema fwschema.Schema) (path.Path, diag.Diagnostics) {
fwPath := path.Empty()

for tfTypeStepIndex, tfTypeStep := range tfType.Steps() {
currentTfTypeSteps := tfType.Steps()[:tfTypeStepIndex+1]
currentTfTypePath := tftypes.NewAttributePathWithSteps(currentTfTypeSteps)
attrType, err := schema.AttributeTypeAtPath(currentTfTypePath)
attrType, err := schema.TypeAtTerraformPath(ctx, currentTfTypePath)

if err != nil {
return path.Empty(), diag.Diagnostics{
Expand All @@ -43,7 +36,7 @@ func attributePath(ctx context.Context, tfType *tftypes.AttributePath, schema Sc
}
}

fwStep, err := fromtftypes.AttributePathStep(ctx, tfTypeStep, attrType)
fwStep, err := AttributePathStep(ctx, tfTypeStep, attrType)

if err != nil {
return path.Empty(), diag.Diagnostics{
Expand Down
@@ -1,13 +1,16 @@
package tfsdk
package fromtftypes_test

import (
"context"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromtftypes"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
testtypes "github.com/hashicorp/terraform-plugin-framework/internal/testing/types"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)
Expand All @@ -17,7 +20,7 @@ func TestAttributePath(t *testing.T) {

testCases := map[string]struct {
tfType *tftypes.AttributePath
schema Schema
schema fwschema.Schema
expected path.Path
expectedDiags diag.Diagnostics
}{
Expand All @@ -31,8 +34,8 @@ func TestAttributePath(t *testing.T) {
},
"AttributeName": {
tfType: tftypes.NewAttributePath().WithAttributeName("test"),
schema: Schema{
Attributes: map[string]Attribute{
schema: tfsdk.Schema{
Attributes: map[string]tfsdk.Attribute{
"test": {
Type: types.StringType,
},
Expand All @@ -42,8 +45,8 @@ func TestAttributePath(t *testing.T) {
},
"AttributeName-nonexistent-attribute": {
tfType: tftypes.NewAttributePath().WithAttributeName("test"),
schema: Schema{
Attributes: map[string]Attribute{
schema: tfsdk.Schema{
Attributes: map[string]tfsdk.Attribute{
"not-test": {
Type: testtypes.StringType{},
},
Expand All @@ -63,8 +66,8 @@ func TestAttributePath(t *testing.T) {
},
"AttributeName-ElementKeyInt": {
tfType: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyInt(1),
schema: Schema{
Attributes: map[string]Attribute{
schema: tfsdk.Schema{
Attributes: map[string]tfsdk.Attribute{
"test": {
Type: types.ListType{
ElemType: types.StringType,
Expand All @@ -76,8 +79,8 @@ func TestAttributePath(t *testing.T) {
},
"AttributeName-ElementKeyValue": {
tfType: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "test-value")),
schema: Schema{
Attributes: map[string]Attribute{
schema: tfsdk.Schema{
Attributes: map[string]tfsdk.Attribute{
"test": {
Type: types.SetType{
ElemType: types.StringType,
Expand All @@ -89,8 +92,8 @@ func TestAttributePath(t *testing.T) {
},
"AttributeName-ElementKeyValue-value-conversion-error": {
tfType: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "test-value")),
schema: Schema{
Attributes: map[string]Attribute{
schema: tfsdk.Schema{
Attributes: map[string]tfsdk.Attribute{
"test": {
Type: types.SetType{
ElemType: testtypes.InvalidType{},
Expand All @@ -112,8 +115,8 @@ func TestAttributePath(t *testing.T) {
},
"ElementKeyInt": {
tfType: tftypes.NewAttributePath().WithElementKeyInt(1),
schema: Schema{
Attributes: map[string]Attribute{
schema: tfsdk.Schema{
Attributes: map[string]tfsdk.Attribute{
"test": {
Type: testtypes.StringType{},
},
Expand All @@ -133,8 +136,8 @@ func TestAttributePath(t *testing.T) {
},
"ElementKeyString": {
tfType: tftypes.NewAttributePath().WithElementKeyString("test"),
schema: Schema{
Attributes: map[string]Attribute{
schema: tfsdk.Schema{
Attributes: map[string]tfsdk.Attribute{
"test": {
Type: testtypes.StringType{},
},
Expand All @@ -154,8 +157,8 @@ func TestAttributePath(t *testing.T) {
},
"ElementKeyValue": {
tfType: tftypes.NewAttributePath().WithElementKeyValue(tftypes.NewValue(tftypes.String, "test-value")),
schema: Schema{
Attributes: map[string]Attribute{
schema: tfsdk.Schema{
Attributes: map[string]tfsdk.Attribute{
"test": {
Type: testtypes.StringType{},
},
Expand All @@ -181,7 +184,7 @@ func TestAttributePath(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()

got, diags := attributePath(context.Background(), testCase.tfType, testCase.schema)
got, diags := fromtftypes.AttributePath(context.Background(), testCase.tfType, testCase.schema)

if diff := cmp.Diff(got, testCase.expected); diff != "" {
t.Errorf("unexpected difference: %s", diff)
Expand Down
7 changes: 7 additions & 0 deletions internal/fwschema/attribute.go
Expand Up @@ -21,6 +21,13 @@ type Attribute interface {
// Equal should return true if the other attribute is exactly equivalent.
Equal(o Attribute) bool

// FrameworkType should return the framework type, whether a direct type
// or nested attributes type, for the attribute.
//
// When tfsdk.Attribute is removed, this should be deprecated and renamed
// to Type() to match other interfaces.
FrameworkType() attr.Type

// GetAttributes should return the nested attributes of an attribute, if
// applicable. This is named differently than Attribute to prevent a
// conflict with the tfsdk.Attribute field name.
Expand Down

0 comments on commit 0ba16ff

Please sign in to comment.