From 12176c957f75b2fe148be97fa85a03bf093cde36 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 3 May 2022 11:26:59 -0400 Subject: [PATCH] diag: Move (Diagnostics).ToTfprotov6Diagnostics() method to internal/toproto6 package (#313) Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/215 --- .changelog/313.txt | 3 + diag/diagnostic_test.go | 31 ------- diag/diagnostics.go | 25 ------ diag/diagnostics_test.go | 78 ------------------ internal/proto6server/serve.go | 20 ++--- internal/proto6server/serve_import.go | 5 +- internal/toproto6/diagnostics.go | 27 ++++++ internal/toproto6/diagnostics_test.go | 114 ++++++++++++++++++++++++++ 8 files changed, 157 insertions(+), 146 deletions(-) create mode 100644 .changelog/313.txt delete mode 100644 diag/diagnostic_test.go create mode 100644 internal/toproto6/diagnostics.go create mode 100644 internal/toproto6/diagnostics_test.go diff --git a/.changelog/313.txt b/.changelog/313.txt new file mode 100644 index 000000000..724dff94a --- /dev/null +++ b/.changelog/313.txt @@ -0,0 +1,3 @@ +```release-note:breaking-change +diag: Removed `Diagnostics` type `ToTfprotov6Diagnostics()` method. This was not intended for usage by provider developers. +``` diff --git a/diag/diagnostic_test.go b/diag/diagnostic_test.go deleted file mode 100644 index dd754d8c5..000000000 --- a/diag/diagnostic_test.go +++ /dev/null @@ -1,31 +0,0 @@ -package diag_test - -import ( - "github.com/hashicorp/terraform-plugin-framework/diag" -) - -var _ diag.Diagnostic = invalidSeverityDiagnostic{} - -type invalidSeverityDiagnostic struct{} - -func (d invalidSeverityDiagnostic) Detail() string { - return "detail for invalid severity diagnostic" -} - -func (d invalidSeverityDiagnostic) Equal(other diag.Diagnostic) bool { - isd, ok := other.(invalidSeverityDiagnostic) - - if !ok { - return false - } - - return isd.Summary() == d.Summary() && isd.Detail() == d.Detail() && isd.Severity() == d.Severity() -} - -func (d invalidSeverityDiagnostic) Severity() diag.Severity { - return diag.SeverityInvalid -} - -func (d invalidSeverityDiagnostic) Summary() string { - return "summary for invalid severity diagnostic" -} diff --git a/diag/diagnostics.go b/diag/diagnostics.go index 8ab488faa..e1bdf3533 100644 --- a/diag/diagnostics.go +++ b/diag/diagnostics.go @@ -1,7 +1,6 @@ package diag import ( - "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -71,27 +70,3 @@ func (diags Diagnostics) HasError() bool { return false } - -// ToTfprotov6Diagnostics converts the diagnostics into the tfprotov6 collection type. -// -// Usage of this method outside the framework is not supported nor considered -// for backwards compatibility promises. -func (diags Diagnostics) ToTfprotov6Diagnostics() []*tfprotov6.Diagnostic { - var results []*tfprotov6.Diagnostic - - for _, diag := range diags { - tfprotov6Diagnostic := &tfprotov6.Diagnostic{ - Detail: diag.Detail(), - Severity: diag.Severity().ToTfprotov6DiagnosticSeverity(), - Summary: diag.Summary(), - } - - if diagWithPath, ok := diag.(DiagnosticWithPath); ok { - tfprotov6Diagnostic.Attribute = diagWithPath.Path() - } - - results = append(results, tfprotov6Diagnostic) - } - - return results -} diff --git a/diag/diagnostics_test.go b/diag/diagnostics_test.go index e05c91ca2..04b7684c9 100644 --- a/diag/diagnostics_test.go +++ b/diag/diagnostics_test.go @@ -5,7 +5,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -519,80 +518,3 @@ func TestDiagnosticsHasError(t *testing.T) { }) } } - -func TestDiagnosticsToTfprotov6Diagnostics(t *testing.T) { - t.Parallel() - - testCases := map[string]struct { - diags diag.Diagnostics - expected []*tfprotov6.Diagnostic - }{ - "nil": { - diags: nil, - expected: nil, - }, - "Diagnostic-SeverityInvalid": { - diags: diag.Diagnostics{ - invalidSeverityDiagnostic{}, - }, - expected: []*tfprotov6.Diagnostic{ - { - Detail: invalidSeverityDiagnostic{}.Detail(), - Severity: tfprotov6.DiagnosticSeverityInvalid, - Summary: invalidSeverityDiagnostic{}.Summary(), - }, - }, - }, - "Diagnostic": { - diags: diag.Diagnostics{ - diag.NewErrorDiagnostic("one summary", "one detail"), - diag.NewWarningDiagnostic("two summary", "two detail"), - }, - expected: []*tfprotov6.Diagnostic{ - { - Detail: "one detail", - Severity: tfprotov6.DiagnosticSeverityError, - Summary: "one summary", - }, - { - Detail: "two detail", - Severity: tfprotov6.DiagnosticSeverityWarning, - Summary: "two summary", - }, - }, - }, - "DiagnosticWithPath": { - diags: diag.Diagnostics{ - diag.NewAttributeErrorDiagnostic(tftypes.NewAttributePath(), "one summary", "one detail"), - diag.NewAttributeWarningDiagnostic(tftypes.NewAttributePath().WithAttributeName("test"), "two summary", "two detail"), - }, - expected: []*tfprotov6.Diagnostic{ - { - Attribute: tftypes.NewAttributePath(), - Detail: "one detail", - Severity: tfprotov6.DiagnosticSeverityError, - Summary: "one summary", - }, - { - Attribute: tftypes.NewAttributePath().WithAttributeName("test"), - Detail: "two detail", - Severity: tfprotov6.DiagnosticSeverityWarning, - Summary: "two summary", - }, - }, - }, - } - - for name, tc := range testCases { - name, tc := name, tc - t.Run(name, func(t *testing.T) { - t.Parallel() - - got := tc.diags.ToTfprotov6Diagnostics() - - if diff := cmp.Diff(got, tc.expected); diff != "" { - t.Errorf("Unexpected response (+wanted, -got): %s", diff) - } - }) - } -} diff --git a/internal/proto6server/serve.go b/internal/proto6server/serve.go index 66e609f12..35c218bac 100644 --- a/internal/proto6server/serve.go +++ b/internal/proto6server/serve.go @@ -101,7 +101,7 @@ func (r getProviderSchemaResponse) toTfprotov6() *tfprotov6.GetProviderSchemaRes ProviderMeta: r.ProviderMeta, ResourceSchemas: r.ResourceSchemas, DataSourceSchemas: r.DataSourceSchemas, - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), } } @@ -240,7 +240,7 @@ type validateProviderConfigResponse struct { func (r validateProviderConfigResponse) toTfprotov6() *tfprotov6.ValidateProviderConfigResponse { return &tfprotov6.ValidateProviderConfigResponse{ PreparedConfig: r.PreparedConfig, - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), } } @@ -353,7 +353,7 @@ type configureProviderResponse struct { func (r configureProviderResponse) toTfprotov6() *tfprotov6.ConfigureProviderResponse { return &tfprotov6.ConfigureProviderResponse{ - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), } } @@ -409,7 +409,7 @@ type validateResourceConfigResponse struct { func (r validateResourceConfigResponse) toTfprotov6() *tfprotov6.ValidateResourceConfigResponse { return &tfprotov6.ValidateResourceConfigResponse{ - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), } } @@ -536,7 +536,7 @@ type upgradeResourceStateResponse struct { func (r upgradeResourceStateResponse) toTfprotov6() *tfprotov6.UpgradeResourceStateResponse { return &tfprotov6.UpgradeResourceStateResponse{ - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), UpgradedState: r.UpgradedState, } } @@ -766,7 +766,7 @@ type readResourceResponse struct { func (r readResourceResponse) toTfprotov6() *tfprotov6.ReadResourceResponse { return &tfprotov6.ReadResourceResponse{ NewState: r.NewState, - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), Private: r.Private, } } @@ -911,7 +911,7 @@ type planResourceChangeResponse struct { func (r planResourceChangeResponse) toTfprotov6() *tfprotov6.PlanResourceChangeResponse { return &tfprotov6.PlanResourceChangeResponse{ PlannedState: r.PlannedState, - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), RequiresReplace: r.RequiresReplace, PlannedPrivate: r.PlannedPrivate, } @@ -1209,7 +1209,7 @@ func (r applyResourceChangeResponse) toTfprotov6() *tfprotov6.ApplyResourceChang return &tfprotov6.ApplyResourceChangeResponse{ NewState: r.NewState, Private: r.Private, - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), } } @@ -1532,7 +1532,7 @@ type validateDataResourceConfigResponse struct { func (r validateDataResourceConfigResponse) toTfprotov6() *tfprotov6.ValidateDataResourceConfigResponse { return &tfprotov6.ValidateDataResourceConfigResponse{ - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), } } @@ -1660,7 +1660,7 @@ type readDataSourceResponse struct { func (r readDataSourceResponse) toTfprotov6() *tfprotov6.ReadDataSourceResponse { return &tfprotov6.ReadDataSourceResponse{ State: r.State, - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), } } diff --git a/internal/proto6server/serve_import.go b/internal/proto6server/serve_import.go index ad3241f7e..6bb1ffdd5 100644 --- a/internal/proto6server/serve_import.go +++ b/internal/proto6server/serve_import.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/logging" + "github.com/hashicorp/terraform-plugin-framework/internal/toproto6" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tftypes" @@ -50,12 +51,12 @@ type importResourceStateResponse struct { func (r importResourceStateResponse) toTfprotov6(ctx context.Context) *tfprotov6.ImportResourceStateResponse { resp := &tfprotov6.ImportResourceStateResponse{ - Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), + Diagnostics: toproto6.Diagnostics(r.Diagnostics), } for _, ir := range r.ImportedResources { irProto6, diags := ir.toTfprotov6(ctx) - resp.Diagnostics = append(resp.Diagnostics, diags.ToTfprotov6Diagnostics()...) + resp.Diagnostics = append(resp.Diagnostics, toproto6.Diagnostics(diags)...) if diags.HasError() { continue } diff --git a/internal/toproto6/diagnostics.go b/internal/toproto6/diagnostics.go new file mode 100644 index 000000000..7829330f9 --- /dev/null +++ b/internal/toproto6/diagnostics.go @@ -0,0 +1,27 @@ +package toproto6 + +import ( + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" +) + +// Diagnostics converts the diagnostics into the tfprotov6 collection type. +func Diagnostics(diagnostics diag.Diagnostics) []*tfprotov6.Diagnostic { + var results []*tfprotov6.Diagnostic + + for _, diagnostic := range diagnostics { + tfprotov6Diagnostic := &tfprotov6.Diagnostic{ + Detail: diagnostic.Detail(), + Severity: diagnostic.Severity().ToTfprotov6DiagnosticSeverity(), + Summary: diagnostic.Summary(), + } + + if diagWithPath, ok := diagnostic.(diag.DiagnosticWithPath); ok { + tfprotov6Diagnostic.Attribute = diagWithPath.Path() + } + + results = append(results, tfprotov6Diagnostic) + } + + return results +} diff --git a/internal/toproto6/diagnostics_test.go b/internal/toproto6/diagnostics_test.go new file mode 100644 index 000000000..47292d700 --- /dev/null +++ b/internal/toproto6/diagnostics_test.go @@ -0,0 +1,114 @@ +package toproto6_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/internal/toproto6" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +func TestDiagnostics(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + diags diag.Diagnostics + expected []*tfprotov6.Diagnostic + }{ + "nil": { + diags: nil, + expected: nil, + }, + "Diagnostic-SeverityInvalid": { + diags: diag.Diagnostics{ + invalidSeverityDiagnostic{}, + }, + expected: []*tfprotov6.Diagnostic{ + { + Detail: invalidSeverityDiagnostic{}.Detail(), + Severity: tfprotov6.DiagnosticSeverityInvalid, + Summary: invalidSeverityDiagnostic{}.Summary(), + }, + }, + }, + "Diagnostic": { + diags: diag.Diagnostics{ + diag.NewErrorDiagnostic("one summary", "one detail"), + diag.NewWarningDiagnostic("two summary", "two detail"), + }, + expected: []*tfprotov6.Diagnostic{ + { + Detail: "one detail", + Severity: tfprotov6.DiagnosticSeverityError, + Summary: "one summary", + }, + { + Detail: "two detail", + Severity: tfprotov6.DiagnosticSeverityWarning, + Summary: "two summary", + }, + }, + }, + "DiagnosticWithPath": { + diags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic(tftypes.NewAttributePath(), "one summary", "one detail"), + diag.NewAttributeWarningDiagnostic(tftypes.NewAttributePath().WithAttributeName("test"), "two summary", "two detail"), + }, + expected: []*tfprotov6.Diagnostic{ + { + Attribute: tftypes.NewAttributePath(), + Detail: "one detail", + Severity: tfprotov6.DiagnosticSeverityError, + Summary: "one summary", + }, + { + Attribute: tftypes.NewAttributePath().WithAttributeName("test"), + Detail: "two detail", + Severity: tfprotov6.DiagnosticSeverityWarning, + Summary: "two summary", + }, + }, + }, + } + + for name, tc := range testCases { + name, tc := name, tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := toproto6.Diagnostics(tc.diags) + + if diff := cmp.Diff(got, tc.expected); diff != "" { + t.Errorf("Unexpected response (+wanted, -got): %s", diff) + } + }) + } +} + +var _ diag.Diagnostic = invalidSeverityDiagnostic{} + +type invalidSeverityDiagnostic struct{} + +func (d invalidSeverityDiagnostic) Detail() string { + return "detail for invalid severity diagnostic" +} + +func (d invalidSeverityDiagnostic) Equal(other diag.Diagnostic) bool { + isd, ok := other.(invalidSeverityDiagnostic) + + if !ok { + return false + } + + return isd.Summary() == d.Summary() && isd.Detail() == d.Detail() && isd.Severity() == d.Severity() +} + +func (d invalidSeverityDiagnostic) Severity() diag.Severity { + return diag.SeverityInvalid +} + +func (d invalidSeverityDiagnostic) Summary() string { + return "summary for invalid severity diagnostic" +}