Skip to content

Commit

Permalink
internal/fwschema: Introduce NestedAttribute interface and cleanup At…
Browse files Browse the repository at this point in the history
…tribute interface methods (#543)

Reference: #132

As part of upcoming effort to split schema functionality into the `datasource`, `provider`, and `resource` packages, there are some internal implementation details which need to be sorted beforehand. Throughout the 0.x versions, schema functionality has been migrated from the `tfsdk` package into the `internal/fwschema` package, but there is some lingering technical debt of duplicate and confusing methods.

This change creates a new `NestedAttribute` (singular) interface which extends the `Attribute` interface with the methods only required for an attribute that does in fact implement nested attributes. The `tfsdk.Attribute` type is updated to support both the `Attribute` and `NestedAttribute` interfaces as it accurately describes the overloaded abstraction. The `NestedAttributes` (plural) interface remains to describe how `tfsdk.Attribute` implements that concepts under its `Attributes` field. The `NestedAttributes` (plural) interface and its implementations will be removed when `tfsdk.Attribute` is removed.

Another aspect of these changes is cleaning up the type describing methods, removing the separate `Attribute` type `FrameworkType()` method and `NestedAttributes` type `AttributeType()` method.

The upcoming split schemas will implement explicit attribute types which only implement `NestedAttribute` (singular) as appropriate.
  • Loading branch information
bflad committed Nov 17, 2022
1 parent b89b71a commit 59a847d
Show file tree
Hide file tree
Showing 27 changed files with 269 additions and 279 deletions.
11 changes: 11 additions & 0 deletions .changelog/543.txt
@@ -0,0 +1,11 @@
```release-note:breaking-change
tfsdk: The `Attribute` type `FrameworkType()` method has been removed. Use the `GetType()` method instead which returns the same information.
```

```release-note:breaking-change
tfsdk: The `Attribute` type `GetAttributes()` method now returns underlying attribute information rather than another interface. Use the `GetNestingMode()` method to determine the nesting mode.
```

```release-note:breaking-change
tfsdk: The `Attribute` type `GetType()` method now returns type information whether the attribute implements the `Type` field or `Attributes` field. Use the `GetAttributes()` and `GetNestingMode()` methods to determine if the `Attributes` field is explicitly being used.
```
19 changes: 3 additions & 16 deletions internal/fwschema/attribute.go
Expand Up @@ -5,10 +5,9 @@ import (
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

// Attribute is the core interface required for implementing Terraform schema
// functionality that can accept a value. This is intended to be the first
// abstraction of tfsdk.Attribute functionality into data source, provider,
// and resource specific functionality.
// Attribute is the core interface required for implementing Terraform
// schema functionality that can accept a value. Refer to NestedAttribute for
// the additional interface that defines nested attributes.
//
// Refer to the internal/fwschema/fwxschema package for optional interfaces
// that define framework-specific functionality, such a plan modification and
Expand All @@ -21,18 +20,6 @@ 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.
GetAttributes() NestedAttributes

// GetDeprecationMessage should return a non-empty string if an attribute
// is deprecated. This is named differently than DeprecationMessage to
// prevent a conflict with the tfsdk.Attribute field name.
Expand Down
16 changes: 16 additions & 0 deletions internal/fwschema/nested_attribute.go
@@ -0,0 +1,16 @@
package fwschema

// NestedAttribute defines a schema attribute that contains nested attributes.
type NestedAttribute interface {
Attribute

// 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.
GetAttributes() UnderlyingAttributes

// GetNestingMode should return the nesting mode (list, map, set, or
// single) of the nested attributes or left unset if this Attribute
// does not represent nested attributes.
GetNestingMode() NestingMode
}
126 changes: 5 additions & 121 deletions internal/fwschema/nested_attributes.go
Expand Up @@ -66,10 +66,6 @@ type NestedAttributes interface {
// interface methods for proper path and data handling.
tftypes.AttributePathStepper

// AttributeType should return the framework type of the nested attributes.
// This method should be deprecated in preference of Type().
AttributeType() attr.Type

// Equal should return true if the other NestedAttributes is equivalent.
Equal(NestedAttributes) bool

Expand All @@ -84,58 +80,12 @@ type NestedAttributes interface {
Type() attr.Type
}

type UnderlyingAttributes map[string]Attribute

func (n UnderlyingAttributes) ApplyTerraform5AttributePathStep(step tftypes.AttributePathStep) (interface{}, error) {
a, ok := step.(tftypes.AttributeName)

if !ok {
return nil, fmt.Errorf("can't apply %T to Attributes", step)
}

res, ok := n[string(a)]

if !ok {
return nil, fmt.Errorf("no attribute %q on Attributes", a)
}

return res, nil
}

// Type returns the framework type of the nested attributes.
func (n UnderlyingAttributes) Type() attr.Type {
attrTypes := map[string]attr.Type{}
for name, attr := range n {
attrTypes[name] = attr.FrameworkType()
}
return types.ObjectType{
AttrTypes: attrTypes,
}
}

type SingleNestedAttributes struct {
UnderlyingAttributes
}

func (s SingleNestedAttributes) ApplyTerraform5AttributePathStep(step tftypes.AttributePathStep) (interface{}, error) {
a, ok := step.(tftypes.AttributeName)

if !ok {
return nil, fmt.Errorf("can't apply %T to Attributes", step)
}

res, ok := s.UnderlyingAttributes[string(a)]

if !ok {
return nil, fmt.Errorf("no attribute %q on Attributes", a)
}

return res, nil
}

// Deprecated: Use Type() instead.
func (s SingleNestedAttributes) AttributeType() attr.Type {
return s.Type()
return s.UnderlyingAttributes.ApplyTerraform5AttributePathStep(step)
}

func (s SingleNestedAttributes) GetAttributes() map[string]Attribute {
Expand All @@ -151,19 +101,7 @@ func (s SingleNestedAttributes) Equal(o NestedAttributes) bool {
if !ok {
return false
}
if len(other.UnderlyingAttributes) != len(s.UnderlyingAttributes) {
return false
}
for k, v := range s.UnderlyingAttributes {
otherV, ok := other.UnderlyingAttributes[k]
if !ok {
return false
}
if !v.Equal(otherV) {
return false
}
}
return true
return s.UnderlyingAttributes.Equal(other.UnderlyingAttributes)
}

// Type returns the framework type of the nested attributes.
Expand All @@ -183,12 +121,6 @@ func (l ListNestedAttributes) GetNestingMode() NestingMode {
return NestingModeList
}

// AttributeType returns an attr.Type corresponding to the nested attributes.
// Deprecated: Use Type() instead.
func (l ListNestedAttributes) AttributeType() attr.Type {
return l.Type()
}

func (l ListNestedAttributes) ApplyTerraform5AttributePathStep(step tftypes.AttributePathStep) (interface{}, error) {
_, ok := step.(tftypes.ElementKeyInt)
if !ok {
Expand All @@ -202,19 +134,7 @@ func (l ListNestedAttributes) Equal(o NestedAttributes) bool {
if !ok {
return false
}
if len(other.UnderlyingAttributes) != len(l.UnderlyingAttributes) {
return false
}
for k, v := range l.UnderlyingAttributes {
otherV, ok := other.UnderlyingAttributes[k]
if !ok {
return false
}
if !v.Equal(otherV) {
return false
}
}
return true
return l.UnderlyingAttributes.Equal(other.UnderlyingAttributes)
}

// Type returns the framework type of the nested attributes.
Expand All @@ -236,12 +156,6 @@ func (s SetNestedAttributes) GetNestingMode() NestingMode {
return NestingModeSet
}

// AttributeType returns an attr.Type corresponding to the nested attributes.
// Deprecated: Use Type() instead.
func (s SetNestedAttributes) AttributeType() attr.Type {
return s.Type()
}

func (s SetNestedAttributes) ApplyTerraform5AttributePathStep(step tftypes.AttributePathStep) (interface{}, error) {
_, ok := step.(tftypes.ElementKeyValue)
if !ok {
Expand All @@ -255,19 +169,7 @@ func (s SetNestedAttributes) Equal(o NestedAttributes) bool {
if !ok {
return false
}
if len(other.UnderlyingAttributes) != len(s.UnderlyingAttributes) {
return false
}
for k, v := range s.UnderlyingAttributes {
otherV, ok := other.UnderlyingAttributes[k]
if !ok {
return false
}
if !v.Equal(otherV) {
return false
}
}
return true
return s.UnderlyingAttributes.Equal(other.UnderlyingAttributes)
}

// Type returns the framework type of the nested attributes.
Expand All @@ -289,12 +191,6 @@ func (m MapNestedAttributes) GetNestingMode() NestingMode {
return NestingModeMap
}

// AttributeType returns an attr.Type corresponding to the nested attributes.
// Deprecated: Use Type() instead.
func (m MapNestedAttributes) AttributeType() attr.Type {
return m.Type()
}

func (m MapNestedAttributes) ApplyTerraform5AttributePathStep(step tftypes.AttributePathStep) (interface{}, error) {
_, ok := step.(tftypes.ElementKeyString)
if !ok {
Expand All @@ -308,19 +204,7 @@ func (m MapNestedAttributes) Equal(o NestedAttributes) bool {
if !ok {
return false
}
if len(other.UnderlyingAttributes) != len(m.UnderlyingAttributes) {
return false
}
for k, v := range m.UnderlyingAttributes {
otherV, ok := other.UnderlyingAttributes[k]
if !ok {
return false
}
if !v.Equal(otherV) {
return false
}
}
return true
return m.UnderlyingAttributes.Equal(other.UnderlyingAttributes)
}

// Type returns the framework type of the nested attributes.
Expand Down
67 changes: 67 additions & 0 deletions internal/fwschema/underlying_attributes.go
@@ -0,0 +1,67 @@
package fwschema

import (
"fmt"

"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

// Ensure UnderlyingAttributes satisfies the expected interfaces.
var _ tftypes.AttributePathStepper = UnderlyingAttributes{}

// UnderlyingAttributes represents attributes under a nested attribute.
type UnderlyingAttributes map[string]Attribute

// ApplyTerraform5AttributePathStep performs an AttributeName step on the
// underlying attributes or returns an error.
func (u UnderlyingAttributes) ApplyTerraform5AttributePathStep(step tftypes.AttributePathStep) (any, error) {
name, ok := step.(tftypes.AttributeName)

if !ok {
return nil, fmt.Errorf("can't apply %T to Attributes", step)
}

attribute, ok := u[string(name)]

if !ok {
return nil, fmt.Errorf("no attribute %q on Attributes", name)
}

return attribute, nil
}

// Equal returns true if all underlying attributes are equal.
func (u UnderlyingAttributes) Equal(o UnderlyingAttributes) bool {
if len(u) != len(o) {
return false
}

for name, uAttribute := range u {
oAttribute, ok := o[name]

if !ok {
return false
}

if !uAttribute.Equal(oAttribute) {
return false
}
}

return true
}

// Type returns the framework type of the underlying attributes.
func (u UnderlyingAttributes) Type() attr.Type {
attrTypes := make(map[string]attr.Type, len(u))

for name, attr := range u {
attrTypes[name] = attr.GetType()
}

return types.ObjectType{
AttrTypes: attrTypes,
}
}
19 changes: 13 additions & 6 deletions internal/fwserver/attribute_plan_modification.go
Expand Up @@ -91,11 +91,18 @@ func AttributeModifyPlan(ctx context.Context, a fwschema.Attribute, req tfsdk.Mo
return
}

if a.GetAttributes() == nil || len(a.GetAttributes().GetAttributes()) == 0 {
nestedAttribute, ok := a.(fwschema.NestedAttribute)

if !ok {
return
}

// Temporarily handle tfsdk.Attribute, which always has a nesting mode, until its removed.
if tfsdkAttribute, ok := a.(tfsdk.Attribute); ok && tfsdkAttribute.GetNestingMode() == fwschema.NestingModeUnknown {
return
}

nm := a.GetAttributes().GetNestingMode()
nm := nestedAttribute.GetNestingMode()
switch nm {
case fwschema.NestingModeList:
configList, diags := coerceListValue(ctx, req.AttributePath, req.AttributeConfig)
Expand Down Expand Up @@ -153,7 +160,7 @@ func AttributeModifyPlan(ctx context.Context, a fwschema.Attribute, req tfsdk.Mo

planAttributes := planObject.Attributes()

for name, attr := range a.GetAttributes().GetAttributes() {
for name, attr := range nestedAttribute.GetAttributes() {
attrConfig, diags := objectAttributeValue(ctx, configObject, name, fwschemadata.DataDescriptionConfiguration)

resp.Diagnostics.Append(diags...)
Expand Down Expand Up @@ -275,7 +282,7 @@ func AttributeModifyPlan(ctx context.Context, a fwschema.Attribute, req tfsdk.Mo

planAttributes := planObject.Attributes()

for name, attr := range a.GetAttributes().GetAttributes() {
for name, attr := range nestedAttribute.GetAttributes() {
attrConfig, diags := objectAttributeValue(ctx, configObject, name, fwschemadata.DataDescriptionConfiguration)

resp.Diagnostics.Append(diags...)
Expand Down Expand Up @@ -397,7 +404,7 @@ func AttributeModifyPlan(ctx context.Context, a fwschema.Attribute, req tfsdk.Mo

planAttributes := planObject.Attributes()

for name, attr := range a.GetAttributes().GetAttributes() {
for name, attr := range nestedAttribute.GetAttributes() {
attrConfig, diags := objectAttributeValue(ctx, configObject, name, fwschemadata.DataDescriptionConfiguration)

resp.Diagnostics.Append(diags...)
Expand Down Expand Up @@ -494,7 +501,7 @@ func AttributeModifyPlan(ctx context.Context, a fwschema.Attribute, req tfsdk.Mo

planAttributes := planObject.Attributes()

for name, attr := range a.GetAttributes().GetAttributes() {
for name, attr := range nestedAttribute.GetAttributes() {
attrConfig, diags := objectAttributeValue(ctx, configObject, name, fwschemadata.DataDescriptionConfiguration)

resp.Diagnostics.Append(diags...)
Expand Down

0 comments on commit 59a847d

Please sign in to comment.