Skip to content

Commit

Permalink
Standardise UUID parsing and error reporting in already tested code
Browse files Browse the repository at this point in the history
  • Loading branch information
@michal-futurice
michal-futurice committed Jul 25, 2024
1 parent 1bd0387 commit 5361741
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 38 deletions.
20 changes: 10 additions & 10 deletions internal/provider/aclmapping/acl_mapping_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ func (r *ACLMappingResource) Create(ctx context.Context, req resource.CreateRequ
return
}

teamID, teamIDDiags := utils.ParseUUID(plan.TeamID.ValueString())
teamID, teamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(teamIDDiags...)

projectID, projectIDDiags := utils.ParseUUID(plan.ProjectID.ValueString())
projectID, projectIDDiags := utils.ParseAttributeUUID(plan.ProjectID.ValueString(), "project_id")
resp.Diagnostics.Append(projectIDDiags...)

if resp.Diagnostics.HasError() {
Expand Down Expand Up @@ -125,10 +125,10 @@ func (r *ACLMappingResource) Read(ctx context.Context, req resource.ReadRequest,
return
}

teamID, teamIDDiags := utils.ParseUUID(state.TeamID.ValueString())
teamID, teamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(teamIDDiags...)

projectID, projectIDDiags := utils.ParseUUID(state.ProjectID.ValueString())
projectID, projectIDDiags := utils.ParseAttributeUUID(state.ProjectID.ValueString(), "project_id")
resp.Diagnostics.Append(projectIDDiags...)

if resp.Diagnostics.HasError() {
Expand Down Expand Up @@ -171,16 +171,16 @@ func (r *ACLMappingResource) Update(ctx context.Context, req resource.UpdateRequ
return
}

oldTeamID, oldTeamIDDiags := utils.ParseUUID(state.TeamID.ValueString())
oldTeamID, oldTeamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(oldTeamIDDiags...)

oldProjectID, oldProjectIDDiags := utils.ParseUUID(state.ProjectID.ValueString())
oldProjectID, oldProjectIDDiags := utils.ParseAttributeUUID(state.ProjectID.ValueString(), "project_id")
resp.Diagnostics.Append(oldProjectIDDiags...)

newTeamID, newTeamIDDiags := utils.ParseUUID(plan.TeamID.ValueString())
newTeamID, newTeamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(newTeamIDDiags...)

newProjectID, newProjectIDDiags := utils.ParseUUID(plan.ProjectID.ValueString())
newProjectID, newProjectIDDiags := utils.ParseAttributeUUID(plan.ProjectID.ValueString(), "project_id")
resp.Diagnostics.Append(newProjectIDDiags...)

if resp.Diagnostics.HasError() {
Expand Down Expand Up @@ -224,10 +224,10 @@ func (r *ACLMappingResource) Delete(ctx context.Context, req resource.DeleteRequ
return
}

teamID, teamIDDiags := utils.ParseUUID(state.TeamID.ValueString())
teamID, teamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(teamIDDiags...)

projectID, projectIDDiags := utils.ParseUUID(state.ProjectID.ValueString())
projectID, projectIDDiags := utils.ParseAttributeUUID(state.ProjectID.ValueString(), "project_id")
resp.Diagnostics.Append(projectIDDiags...)

if resp.Diagnostics.HasError() {
Expand Down
35 changes: 27 additions & 8 deletions internal/provider/project/project_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import (
"context"
"fmt"

dtrack "github.com/futurice/dependency-track-client-go"
"github.com/google/uuid"
"github.com/futurice/terraform-provider-dependencytrack/internal/utils"

dtrack "github.com/futurice/dependency-track-client-go"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/resource"
Expand Down Expand Up @@ -120,13 +120,15 @@ func (r *ProjectResource) Create(ctx context.Context, req resource.CreateRequest
var plan, state ProjectResourceModel

resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)

if resp.Diagnostics.HasError() {
return
}

dtProject, diags := TFProjectToDTProject(ctx, plan)
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return
}

respProject, err := r.client.Project.Create(ctx, dtProject)
if err != nil {
Expand Down Expand Up @@ -157,12 +159,17 @@ func (r *ProjectResource) Read(ctx context.Context, req resource.ReadRequest, re
var diags diag.Diagnostics

resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
if resp.Diagnostics.HasError() {
return
}

projectID, projectIDDiags := utils.ParseAttributeUUID(state.ID.ValueString(), "id")
resp.Diagnostics.Append(projectIDDiags...)
if resp.Diagnostics.HasError() {
return
}

respProject, err := r.client.Project.Get(ctx, uuid.MustParse(state.ID.ValueString()))
respProject, err := r.client.Project.Get(ctx, projectID)
if err != nil {
if apiErr, ok := err.(*dtrack.APIError); ok && apiErr.StatusCode == 404 {
resp.State.RemoveResource(ctx)
Expand All @@ -184,13 +191,15 @@ func (r *ProjectResource) Update(ctx context.Context, req resource.UpdateRequest

resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
resp.Diagnostics.Append(req.State.Get(ctx, &state)...)

if resp.Diagnostics.HasError() {
return
}

dtProject, diags := TFProjectToDTProject(ctx, plan)
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return
}

respProject, err := r.client.Project.Update(ctx, dtProject)
if err != nil {
Expand All @@ -211,12 +220,17 @@ func (r *ProjectResource) Delete(ctx context.Context, req resource.DeleteRequest
var state ProjectResourceModel

resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
if resp.Diagnostics.HasError() {
return
}

projectID, projectIDDiags := utils.ParseAttributeUUID(state.ID.ValueString(), "id")
resp.Diagnostics.Append(projectIDDiags...)
if resp.Diagnostics.HasError() {
return
}

err := r.client.Project.Delete(ctx, uuid.MustParse(state.ID.ValueString()))
err := r.client.Project.Delete(ctx, projectID)
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to delete project, got error: %s", err))
return
Expand Down Expand Up @@ -255,6 +269,7 @@ func DTProjectToTFProject(ctx context.Context, dtProject dtrack.Project) (Projec

func TFProjectToDTProject(ctx context.Context, tfProject ProjectResourceModel) (dtrack.Project, diag.Diagnostics) {
var diags diag.Diagnostics

project := dtrack.Project{
Name: tfProject.Name.ValueString(),
Classifier: tfProject.Classifier.ValueString(),
Expand All @@ -263,11 +278,15 @@ func TFProjectToDTProject(ctx context.Context, tfProject ProjectResourceModel) (
}

if tfProject.ID.ValueString() != "" {
project.UUID = uuid.MustParse(tfProject.ID.ValueString())
projectID, projectIDDiags := utils.ParseAttributeUUID(tfProject.ID.ValueString(), "id")
project.UUID = projectID
diags.Append(projectIDDiags...)
}

if !tfProject.ParentID.IsNull() {
project.ParentRef = &dtrack.ParentRef{UUID: uuid.MustParse(tfProject.ParentID.ValueString())}
parentProjectID, parentProjectIDDiags := utils.ParseAttributeUUID(tfProject.ParentID.ValueString(), "parent_id")
project.ParentRef = &dtrack.ParentRef{UUID: parentProjectID}
diags.Append(parentProjectIDDiags...)
}

return project, diags
Expand Down
16 changes: 4 additions & 12 deletions internal/provider/team/team_data_source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ import (
"context"
"fmt"

dtrack "github.com/futurice/dependency-track-client-go"
"github.com/google/uuid"
"github.com/futurice/terraform-provider-dependencytrack/internal/utils"

dtrack "github.com/futurice/dependency-track-client-go"
"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/types"
)

Expand Down Expand Up @@ -90,19 +89,12 @@ func (d *TeamDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
var model TeamDataSourceModel

resp.Diagnostics.Append(req.Config.Get(ctx, &model)...)

if resp.Diagnostics.HasError() {
return
}

teamID, err := uuid.Parse(model.ID.ValueString())
if err != nil {
resp.Diagnostics.AddAttributeError(path.Root("id"),
"Invalid team ID",
"Team ID has to be a valid UUID.",
)
}

teamID, teamIDDiags := utils.ParseAttributeUUID(model.ID.ValueString(), "id")
resp.Diagnostics.Append(teamIDDiags...)
if resp.Diagnostics.HasError() {
return
}
Expand Down
4 changes: 2 additions & 2 deletions internal/provider/team/team_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (r *TeamResource) Read(ctx context.Context, req resource.ReadRequest, resp
return
}

teamID, teamIDDiags := utils.ParseUUID(state.ID.ValueString())
teamID, teamIDDiags := utils.ParseAttributeUUID(state.ID.ValueString(), "id")
resp.Diagnostics.Append(teamIDDiags...)
if resp.Diagnostics.HasError() {
return
Expand Down Expand Up @@ -206,7 +206,7 @@ func TFTeamToDTTeam(ctx context.Context, tfTeam TeamResourceModel) (dtrack.Team,
}

if tfTeam.ID.ValueString() != "" {
teamID, teamIDDiags := utils.ParseUUID(tfTeam.ID.ValueString())
teamID, teamIDDiags := utils.ParseAttributeUUID(tfTeam.ID.ValueString(), "id")
team.UUID = teamID
diags.Append(teamIDDiags...)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/teamapikey/team_api_key_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (r *TeamAPIKeyResource) Create(ctx context.Context, req resource.CreateRequ
return
}

teamID, teamIDDiags := utils.ParseUUID(plan.TeamID.ValueString())
teamID, teamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(teamIDDiags...)
if resp.Diagnostics.HasError() {
return
Expand Down
10 changes: 5 additions & 5 deletions internal/provider/teampermission/team_permission_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (r *TeamPermissionResource) Create(ctx context.Context, req resource.Create
return
}

teamID, teamIDDiags := utils.ParseUUID(plan.TeamID.ValueString())
teamID, teamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(teamIDDiags...)
if resp.Diagnostics.HasError() {
return
Expand Down Expand Up @@ -129,7 +129,7 @@ func (r *TeamPermissionResource) Read(ctx context.Context, req resource.ReadRequ
return
}

teamID, teamIDDiags := utils.ParseUUID(state.TeamID.ValueString())
teamID, teamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(teamIDDiags...)
if resp.Diagnostics.HasError() {
return
Expand Down Expand Up @@ -171,10 +171,10 @@ func (r *TeamPermissionResource) Update(ctx context.Context, req resource.Update
return
}

newTeamID, newTeamIDDiags := utils.ParseUUID(plan.TeamID.ValueString())
newTeamID, newTeamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(newTeamIDDiags...)

oldTeamID, oldTeamIDDiags := utils.ParseUUID(state.TeamID.ValueString())
oldTeamID, oldTeamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(oldTeamIDDiags...)

if resp.Diagnostics.HasError() {
Expand Down Expand Up @@ -216,7 +216,7 @@ func (r *TeamPermissionResource) Delete(ctx context.Context, req resource.Delete
return
}

teamID, teamIDDiags := utils.ParseUUID(state.TeamID.ValueString())
teamID, teamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id")
resp.Diagnostics.Append(teamIDDiags...)
if resp.Diagnostics.HasError() {
return
Expand Down
14 changes: 14 additions & 0 deletions internal/utils/terraform.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
)

// ParseUUID parses the UUID value returning a possible error as Diagnostics instead of error.
Expand All @@ -18,3 +19,16 @@ func ParseUUID(uuidString string) (uuid.UUID, diag.Diagnostics) {

return id, diags
}

// ParseAttributeUUID works just as ParseUUID but returns any errors as attribute errors on the specified attribute path.
func ParseAttributeUUID(uuidString string, attributePath string) (uuid.UUID, diag.Diagnostics) {
var diags diag.Diagnostics

id, err := uuid.Parse(uuidString)
if err != nil {
diags.AddAttributeError(path.Root(attributePath), "Invalid UUID", fmt.Sprintf("Failed to parse string [%s] as UUID: %v", uuidString, err))
return uuid.UUID{}, diags
}

return id, diags
}
27 changes: 27 additions & 0 deletions internal/utils/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package utils_test
import (
"github.com/futurice/terraform-provider-dependencytrack/internal/utils"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
"testing"
)

Expand All @@ -26,3 +28,28 @@ func TestParseUUID_invalid(t *testing.T) {
t.Errorf("Error expected, but received none")
}
}

func TestParseAttributeUUID_basic(t *testing.T) {
testUUID := uuid.MustParse("8ffb30fb-77e6-4886-9f32-ff142f9bf90b")
result, diags := utils.ParseAttributeUUID(testUUID.String(), "id")

if result != testUUID {
t.Errorf("Parsed UUID [%s] is different than expected [%s]", result, testUUID)
}

if diags.HasError() {
t.Errorf("Unexpected error: %v", diags)
}
}

func TestParseAttributeUUID_invalid(t *testing.T) {
_, diags := utils.ParseAttributeUUID("not-an-UUID", "id")

if !diags.HasError() {
t.Errorf("Error expected, but received none")
}

if !diags.Contains(diag.NewAttributeErrorDiagnostic(path.Root("id"), "Invalid UUID", "Failed to parse string [not-an-UUID] as UUID: invalid UUID length: 11")) {
t.Errorf("Diags do not contain the expected attribute error: %v", diags)
}
}

0 comments on commit 5361741

Please sign in to comment.