Skip to content

Commit

Permalink
chore: Improves schema Description / MarkdownDescription (#2605)
Browse files Browse the repository at this point in the history
* add conversion calls

* UpdateSchemaDescription and UpdateDSSchemaDescription

* only update empty Description

* fix Description

* set MarkdownDescripton if empty

* remove redundant Description

* project_ip_access_list

* case SingleNestedBlock

* validate blocks

* reduce error message len

* use reflection

* don't allow both descriptions

* unify description checks

* simplify attr names

* refactor tests

* only 1 func UpdateSchemaDescription

* unit testsg
  • Loading branch information
lantoli authored Sep 20, 2024
1 parent 436096d commit 0aeb0df
Show file tree
Hide file tree
Showing 36 changed files with 332 additions and 143 deletions.
2 changes: 1 addition & 1 deletion docs/data-sources/encryption_at_rest_private_endpoints.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ output "number_of_endpoints" {

### Required

- `cloud_provider` (String) Human-readable label that identifies the cloud provider for the private endpoints to return.
- `cloud_provider` (String) Label that identifies the cloud provider of the private endpoint.
- `project_id` (String) Unique 24-hexadecimal digit string that identifies your project.

### Read-Only
Expand Down
74 changes: 74 additions & 0 deletions internal/common/conversion/schema_description.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package conversion

import (
"reflect"

dsschema "github.com/hashicorp/terraform-plugin-framework/datasource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
)

func UpdateSchemaDescription[T schema.Schema | dsschema.Schema](s *T) {
UpdateAttr(s)
}

// UpdateAttr is exported for testing purposes only and should not be used directly.
func UpdateAttr(attr any) {
ptr := reflect.ValueOf(attr)
if ptr.Kind() != reflect.Ptr {
panic("not ptr, please fix caller")
}
v := ptr.Elem()
if v.Kind() != reflect.Struct {
panic("not struct, please fix caller")
}
updateDesc(v)
updateMap(v, "Attributes")
updateMap(v, "Blocks")
updateNested(v, "NestedObject")
}

func updateDesc(v reflect.Value) {
fDescr, fMDDescr := v.FieldByName("Description"), v.FieldByName("MarkdownDescription")
if !fDescr.IsValid() || !fMDDescr.IsValid() {
return
}
if !fDescr.CanSet() || fDescr.Kind() != reflect.String ||
!fMDDescr.CanSet() || fMDDescr.Kind() != reflect.String {
panic("invalid desc fields, please fix caller")
}
strDescr, strMDDescr := fDescr.String(), fMDDescr.String()
if strDescr != "" && strMDDescr != "" {
panic("both descriptions exist, please fix caller: " + strDescr)
}
if strDescr == "" {
fDescr.SetString(fMDDescr.String())
} else {
fMDDescr.SetString(fDescr.String())
}
}

func updateMap(v reflect.Value, mapName string) {
f := v.FieldByName(mapName)
if !f.IsValid() {
return
}
if f.Kind() != reflect.Map {
panic("not map, please fix caller: " + mapName)
}
for _, k := range f.MapKeys() {
v := f.MapIndex(k).Elem()
newPtr := reflect.New(v.Type())
newPtr.Elem().Set(v)
UpdateAttr(newPtr.Interface())
f.SetMapIndex(k, newPtr.Elem())
}
}

func updateNested(v reflect.Value, nestedName string) {
f := v.FieldByName(nestedName)
if !f.IsValid() {
return
}
ptr := f.Addr()
UpdateAttr(ptr.Interface())
}
166 changes: 166 additions & 0 deletions internal/common/conversion/schema_description_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
package conversion_test

import (
"testing"

"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
"github.com/stretchr/testify/assert"
)

func TestUpdateSchemaDescription(t *testing.T) {
s := schema.Schema{
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Computed: true,
},
"project_id": schema.StringAttribute{
Required: true,
MarkdownDescription: "mddesc project_id",
},
"nested": schema.ListNestedAttribute{
Computed: true,
MarkdownDescription: "mddesc nested",
NestedObject: schema.NestedAttributeObject{
Attributes: map[string]schema.Attribute{
"attr1": schema.StringAttribute{
Description: "desc attr1",
Computed: true,
},
"attr2": schema.StringAttribute{
MarkdownDescription: "mddesc attr2",
Computed: true,
},
},
},
},
},
Blocks: map[string]schema.Block{
"list": schema.ListNestedBlock{
MarkdownDescription: "mddesc list",
NestedObject: schema.NestedBlockObject{
Attributes: map[string]schema.Attribute{
"attr3": schema.BoolAttribute{
Optional: true,
Computed: true,
MarkdownDescription: "mddesc attr3",
},
},
},
},
"set": schema.SetNestedBlock{
MarkdownDescription: "mddesc set",
NestedObject: schema.NestedBlockObject{
Attributes: map[string]schema.Attribute{
"attr4": schema.StringAttribute{
Optional: true,
MarkdownDescription: "mddesc attr4",
},
"attr5": schema.Int64Attribute{
Required: true,
MarkdownDescription: "mddesc attr5",
},
},
},
},
},
}

expected := schema.Schema{
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Computed: true,
},
"project_id": schema.StringAttribute{
Required: true,
Description: "mddesc project_id",
MarkdownDescription: "mddesc project_id",
},
"nested": schema.ListNestedAttribute{
Computed: true,
Description: "mddesc nested",
MarkdownDescription: "mddesc nested",
NestedObject: schema.NestedAttributeObject{
Attributes: map[string]schema.Attribute{
"attr1": schema.StringAttribute{
Description: "desc attr1",
MarkdownDescription: "desc attr1",
Computed: true,
},
"attr2": schema.StringAttribute{
Description: "mddesc attr2",
MarkdownDescription: "mddesc attr2",
Computed: true,
},
},
},
},
},
Blocks: map[string]schema.Block{
"list": schema.ListNestedBlock{
Description: "mddesc list",
MarkdownDescription: "mddesc list",
NestedObject: schema.NestedBlockObject{
Attributes: map[string]schema.Attribute{
"attr3": schema.BoolAttribute{
Optional: true,
Computed: true,
Description: "mddesc attr3",
MarkdownDescription: "mddesc attr3",
},
},
},
},
"set": schema.SetNestedBlock{
Description: "mddesc set",
MarkdownDescription: "mddesc set",
NestedObject: schema.NestedBlockObject{
Attributes: map[string]schema.Attribute{
"attr4": schema.StringAttribute{
Optional: true,
Description: "mddesc attr4",
MarkdownDescription: "mddesc attr4",
},
"attr5": schema.Int64Attribute{
Required: true,
Description: "mddesc attr5",
MarkdownDescription: "mddesc attr5",
},
},
},
},
},
}

conversion.UpdateSchemaDescription(&s)
assert.Equal(t, expected, s)
}

func TestUpdateAttrPanic(t *testing.T) {
testCases := map[string]any{
"not ptr, please fix caller": "no ptr",
"not struct, please fix caller": conversion.Pointer("no struct"),
"invalid desc fields, please fix caller": conversion.Pointer(struct {
Description int
MarkdownDescription int
}{}),
"both descriptions exist, please fix caller: description": conversion.Pointer(struct {
Description string
MarkdownDescription string
}{
Description: "description",
MarkdownDescription: "markdown description",
}),
"not map, please fix caller: Attributes": conversion.Pointer(struct {
Attributes string
}{}),
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
assert.PanicsWithValue(t, name, func() {
conversion.UpdateAttr(tc)
})
})
}
}
57 changes: 57 additions & 0 deletions internal/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@ package provider_test

import (
"context"
"fmt"
"testing"

"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/diag"
providerfw "github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/provider/schema"
"github.com/hashicorp/terraform-plugin-framework/resource"

"github.com/mongodb/terraform-provider-mongodbatlas/internal/provider"
)

Expand All @@ -30,6 +34,7 @@ func TestResourceSchemas(t *testing.T) {
schemaRequest := resource.SchemaRequest{}
schemaResponse := &resource.SchemaResponse{}
res.Schema(ctx, schemaRequest, schemaResponse)
validateDocumentation(metadataRes.TypeName, schemaResponse)

if schemaResponse.Diagnostics.HasError() {
t.Fatalf("Schema method diagnostics: %+v", schemaResponse.Diagnostics)
Expand Down Expand Up @@ -62,6 +67,7 @@ func TestDataSourceSchemas(t *testing.T) {
schemaRequest := datasource.SchemaRequest{}
schemaResponse := &datasource.SchemaResponse{}
res.Schema(ctx, schemaRequest, schemaResponse)
validateDSDocumentation(metadataRes.TypeName, schemaResponse)

if schemaResponse.Diagnostics.HasError() {
t.Fatalf("Schema method diagnostics: %+v", schemaResponse.Diagnostics)
Expand All @@ -73,3 +79,54 @@ func TestDataSourceSchemas(t *testing.T) {
})
}
}

func validateDocumentation(name string, resp *resource.SchemaResponse) {
checkDescriptor(name, resp.Schema, &resp.Diagnostics)
validateAttributes(name, resp.Schema.GetAttributes(), &resp.Diagnostics)
validateBlocks(name, resp.Schema.GetBlocks(), &resp.Diagnostics)
}

func validateDSDocumentation(name string, resp *datasource.SchemaResponse) {
checkDescriptor(name, resp.Schema, &resp.Diagnostics)
validateAttributes(name, resp.Schema.GetAttributes(), &resp.Diagnostics)
validateBlocks(name, resp.Schema.GetBlocks(), &resp.Diagnostics)
}

func validateAttribute(name string, attr schema.Attribute, diagnostics *diag.Diagnostics) {
checkDescriptor(name, attr, diagnostics)
if nested, ok := attr.(schema.NestedAttribute); ok {
validateAttributes(name, nested.GetNestedObject().GetAttributes(), diagnostics)
}
}

func validateBlock(name string, block schema.Block, diagnostics *diag.Diagnostics) {
checkDescriptor(name, block, diagnostics)
validateAttributes(name, block.GetNestedObject().GetAttributes(), diagnostics)
validateBlocks(name, block.GetNestedObject().GetBlocks(), diagnostics)
}

func validateAttributes[T schema.Attribute](name string, attrs map[string]T, diagnostics *diag.Diagnostics) {
for k, v := range attrs {
validateAttribute(name+"."+k, v, diagnostics)
}
}

func validateBlocks[T schema.Block](name string, blocks map[string]T, diagnostics *diag.Diagnostics) {
for k, v := range blocks {
validateBlock(name+"."+k, v, diagnostics)
}
}

type descriptor interface {
GetDescription() string
GetMarkdownDescription() string
}

func checkDescriptor(name string, d descriptor, diagnostics *diag.Diagnostics) {
if d.GetDescription() != d.GetMarkdownDescription() {
diagnostics.Append(diag.NewErrorDiagnostic(
"Conflicting Attribute Description",
fmt.Sprintf("Description and MarkdownDescription differ for %q.", name),
))
}
}
2 changes: 2 additions & 0 deletions internal/service/controlplaneipaddresses/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/config"
)

Expand All @@ -26,6 +27,7 @@ type controlPlaneIPAddressesDS struct {

func (d *controlPlaneIPAddressesDS) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) {
resp.Schema = DataSourceSchema(ctx)
conversion.UpdateSchemaDescription(&resp.Schema)
}

func (d *controlPlaneIPAddressesDS) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) {
Expand Down
Loading

0 comments on commit 0aeb0df

Please sign in to comment.