Skip to content

Fix pointer handling issues across codebase for better Go idioms #805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jun 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changes/unreleased/fixed_pointer_handling.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
kind: fixed
body: Fix pointer handling issues across codebase for better Go idioms
custom:
Issue: 804
6 changes: 6 additions & 0 deletions internal/customtypes/uuid.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,28 @@ import (
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
)

// NewUUIDNull returns a UUID representing a null value.
func NewUUIDNull() UUID {
return UUID{
StringValue: basetypes.NewStringNull(),
}
}

// NewUUIDUnknown returns a UUID representing an unknown value.
func NewUUIDUnknown() UUID {
return UUID{
StringValue: basetypes.NewStringUnknown(),
}
}

// NewUUIDValue returns a UUID initialized with the given string value.
func NewUUIDValue(value string) UUID {
return UUID{
StringValue: basetypes.NewStringValue(value),
}
}

// NewUUIDPointerValue returns a UUID from a string pointer, or null if the pointer is nil.
func NewUUIDPointerValue(value *string) UUID {
if value == nil {
return NewUUIDNull()
Expand All @@ -34,10 +38,12 @@ func NewUUIDPointerValue(value *string) UUID {
return NewUUIDValue(*value)
}

// NewUUIDValueMust returns a UUID from a string value and validates it as a UUID.
func NewUUIDValueMust(value string) (UUID, diag.Diagnostics) {
return NewUUIDValue(value).ValueUUID()
}

// NewUUIDPointerValueMust returns a UUID from a string pointer and validates it as a UUID.
func NewUUIDPointerValueMust(value *string) (UUID, diag.Diagnostics) {
return NewUUIDValue(*value).ValueUUID()
}
9 changes: 9 additions & 0 deletions internal/helpers/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,12 @@ func StringPtr(s string) *string {
func IsKnown(value attr.Value) bool {
return !value.IsNull() && !value.IsUnknown()
}

// BoolPointer returns a pointer to the bool value if the BoolValue is not null and not unknown, otherwise returns nil.
// This helper reduces boilerplate for null/unknown checks when converting BoolValue to *bool.
func BoolPointer(v basetypes.BoolValue) *bool {
if !v.IsNull() && !v.IsUnknown() {
return v.ValueBoolPointer()
}
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ func (client *client) GetAdminApplication(ctx context.Context, clientId string)
return nil, fmt.Errorf("failed to get admin app %s: %w", clientId, err)
}

return &adminApp, nil
// Returning pointer to local variable is acceptable for small DTOs
return &adminApp, err
}

func (client *client) RegisterAdminApplication(ctx context.Context, clientId string) (*adminManagementApplicationDto, error) {
Expand All @@ -59,7 +60,8 @@ func (client *client) RegisterAdminApplication(ctx context.Context, clientId str
return nil, fmt.Errorf("failed to register admin app %s: %w", clientId, err)
}

return &adminApp, nil
// Returning pointer to local variable is acceptable for small DTOs
return &adminApp, err
}

func (client *client) UnregisterAdminApplication(ctx context.Context, clientId string) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (client *Client) GetGatewayCluster(ctx context.Context) (*GatewayClusterDto
return &gatewayCluster, nil
}

func (client *Client) GetAnalyticsDataExport(ctx context.Context) (*[]AnalyticsDataDto, error) {
func (client *Client) GetAnalyticsDataExport(ctx context.Context) ([]AnalyticsDataDto, error) {
// Get the gateway cluster
gatewayCluster, err := client.GetGatewayCluster(ctx)
if err != nil {
Expand Down Expand Up @@ -83,7 +83,7 @@ func (client *Client) GetAnalyticsDataExport(ctx context.Context) (*[]AnalyticsD
return nil, fmt.Errorf("failed to get analytics data export: %w", err)
}

return &adr.Value, nil
return adr.Value, nil
}

func getAnalyticsUrlMap() map[string]string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,18 +189,10 @@ func (d *AnalyticsExportDataSource) Read(ctx context.Context, req datasource.Rea
)
return
}
if analyticsDataExport == nil {
resp.Diagnostics.AddError(
"Analytics data export not found",
"Unable to find analytics data export with the specified ID",
)
return
}

// Map the response to the model
// Map each analytics data export item to a model
exports := make([]AnalyticsDataModel, 0, len(*analyticsDataExport))
for _, export := range *analyticsDataExport {
exports := make([]AnalyticsDataModel, 0, len(analyticsDataExport))
for _, export := range analyticsDataExport {
if model := convertDtoToModel(&export); model != nil {
exports = append(exports, *model)
}
Expand Down
6 changes: 3 additions & 3 deletions internal/services/connection/resource_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (r *Resource) Configure(ctx context.Context, req resource.ConfigureRequest,
func (r *Resource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
ctx, exitContext := helpers.EnterRequestContext(ctx, r.TypeInfo, req)
defer exitContext()
var plan *ResourceModel
var plan ResourceModel

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

Expand Down Expand Up @@ -246,11 +246,11 @@ func (r *Resource) Update(ctx context.Context, req resource.UpdateRequest, resp
ctx, exitContext := helpers.EnterRequestContext(ctx, r.TypeInfo, req)
defer exitContext()

var plan *ResourceModel
var plan ResourceModel

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

var state *ResourceModel
var state ResourceModel
resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
if resp.Diagnostics.HasError() {
return
Expand Down
6 changes: 3 additions & 3 deletions internal/services/data_record/resource_data_record.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (r *DataRecordResource) Create(ctx context.Context, req resource.CreateRequ
func (r *DataRecordResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
ctx, exitContext := helpers.EnterRequestContext(ctx, r.TypeInfo, req)
defer exitContext()
var state *DataRecordResourceModel
var state DataRecordResourceModel

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

Expand Down Expand Up @@ -198,11 +198,11 @@ func (r *DataRecordResource) Update(ctx context.Context, req resource.UpdateRequ
ctx, exitContext := helpers.EnterRequestContext(ctx, r.TypeInfo, req)
defer exitContext()

var plan *DataRecordResourceModel
var plan DataRecordResourceModel

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

var state *DataRecordResourceModel
var state DataRecordResourceModel
resp.Diagnostics.Append(req.State.Get(ctx, &state)...)

if resp.Diagnostics.HasError() {
Expand Down
4 changes: 2 additions & 2 deletions internal/services/environment/datasource_environments.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (d *EnvironmentsDataSource) Schema(ctx context.Context, req datasource.Sche
MarkdownDescription: "Gives you the ability to create environments that are updated first. This allows you to experience and validate scenarios that are important to you before any updates reach your business-critical applications. See [more](https://learn.microsoft.com/en-us/power-platform/admin/early-release).",
Computed: true,
},
"billing_policy_id": &schema.StringAttribute{
"billing_policy_id": schema.StringAttribute{
MarkdownDescription: "Billing policy id (guid) for pay-as-you-go environments using Azure subscription billing",
Computed: true,
},
Expand Down Expand Up @@ -203,7 +203,7 @@ func (d *EnvironmentsDataSource) Schema(ctx context.Context, req datasource.Sche
MarkdownDescription: "URL of the linked D365 app",
Computed: true,
},
"currency_code": &schema.StringAttribute{
"currency_code": schema.StringAttribute{
MarkdownDescription: "Currency name (EUR, USE, GBP etc.)",
Computed: true,
},
Expand Down
12 changes: 6 additions & 6 deletions internal/services/solution_checker_rules/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,20 @@ import (
"github.com/microsoft/terraform-provider-power-platform/internal/services/environment"
)

type client struct {
type Client struct {
Api *api.Client
environmentClient environment.Client
}

func newSolutionCheckerRulesClient(apiClient *api.Client) client {
return client{
func NewSolutionCheckerRulesClient(apiClient *api.Client) *Client {
return &Client{
Api: apiClient,
environmentClient: environment.NewEnvironmentClient(apiClient),
}
}

// Data transfer objects.
type ruleDto struct {
type RuleDto struct {
Code string `json:"code"`
Description string `json:"description"`
Summary string `json:"summary"`
Expand All @@ -39,7 +39,7 @@ type ruleDto struct {
Severity int `json:"severity"`
}

func (c *client) GetSolutionCheckerRules(ctx context.Context, environmentId string) ([]ruleDto, error) {
func (c *Client) GetSolutionCheckerRules(ctx context.Context, environmentId string) ([]RuleDto, error) {
env, err := c.environmentClient.GetEnvironment(ctx, environmentId)
if err != nil {
return nil, fmt.Errorf("failed to get environment details for %s: %w", environmentId, err)
Expand Down Expand Up @@ -68,7 +68,7 @@ func (c *client) GetSolutionCheckerRules(ctx context.Context, environmentId stri
queryParams.Add("api-version", "2.0")
rulesUrl.RawQuery = queryParams.Encode()

var rules []ruleDto
var rules []RuleDto
_, err = c.Api.Execute(ctx, nil, "GET", rulesUrl.String(), nil, nil, []int{http.StatusOK}, &rules)
if err != nil {
return nil, fmt.Errorf("failed to get solution checker rules: %w", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ func (d *DataSource) Configure(ctx context.Context, req datasource.ConfigureRequ
)
return
}

// Additional safety check for nil client
if client != nil {
d.SolutionCheckerRulesClient = newSolutionCheckerRulesClient(client.Api)
d.SolutionCheckerRulesClient = NewSolutionCheckerRulesClient(client.Api)
} else {
tflog.Warn(ctx, "Client is nil. Datasource will not be fully configured.", map[string]any{
"datasource": d.FullTypeName(),
Expand Down
4 changes: 2 additions & 2 deletions internal/services/solution_checker_rules/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

type DataSource struct {
helpers.TypeInfo
SolutionCheckerRulesClient client
SolutionCheckerRulesClient *Client
}

type DataSourceModel struct {
Expand All @@ -34,7 +34,7 @@ type RuleModel struct {
}

// Helper function to convert from DTO to Model.
func convertFromRuleDto(rule ruleDto) RuleModel {
func convertFromRuleDto(rule RuleDto) RuleModel {
return RuleModel{
Code: types.StringValue(rule.Code),
Description: types.StringValue(rule.Description),
Expand Down
37 changes: 10 additions & 27 deletions internal/services/tenant_settings/dto.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
"github.com/microsoft/terraform-provider-power-platform/internal/customtypes"
"github.com/microsoft/terraform-provider-power-platform/internal/helpers"
)

type tenantDto struct {
Expand Down Expand Up @@ -133,33 +134,15 @@ type tenantSettingsDto struct {
func convertFromTenantSettingsModel(ctx context.Context, tenantSettings TenantSettingsResourceModel) (tenantSettingsDto, error) {
tenantSettingsDto := tenantSettingsDto{}

if !tenantSettings.WalkMeOptOut.IsNull() && !tenantSettings.WalkMeOptOut.IsUnknown() {
tenantSettingsDto.WalkMeOptOut = tenantSettings.WalkMeOptOut.ValueBoolPointer()
}
if !tenantSettings.DisableNPSCommentsReachout.IsNull() && !tenantSettings.DisableNPSCommentsReachout.IsUnknown() {
tenantSettingsDto.DisableNPSCommentsReachout = tenantSettings.DisableNPSCommentsReachout.ValueBoolPointer()
}
if !tenantSettings.DisableNewsletterSendout.IsNull() && !tenantSettings.DisableNewsletterSendout.IsUnknown() {
tenantSettingsDto.DisableNewsletterSendout = tenantSettings.DisableNewsletterSendout.ValueBoolPointer()
}
if !tenantSettings.DisableEnvironmentCreationByNonAdminUsers.IsNull() && !tenantSettings.DisableEnvironmentCreationByNonAdminUsers.IsUnknown() {
tenantSettingsDto.DisableEnvironmentCreationByNonAdminUsers = tenantSettings.DisableEnvironmentCreationByNonAdminUsers.ValueBoolPointer()
}
if !tenantSettings.DisablePortalsCreationByNonAdminUsers.IsNull() && !tenantSettings.DisablePortalsCreationByNonAdminUsers.IsUnknown() {
tenantSettingsDto.DisablePortalsCreationByNonAdminUsers = tenantSettings.DisablePortalsCreationByNonAdminUsers.ValueBoolPointer()
}
if !tenantSettings.DisableSurveyFeedback.IsNull() && !tenantSettings.DisableSurveyFeedback.IsUnknown() {
tenantSettingsDto.DisableSurveyFeedback = tenantSettings.DisableSurveyFeedback.ValueBoolPointer()
}
if !tenantSettings.DisableTrialEnvironmentCreationByNonAdminUsers.IsNull() && !tenantSettings.DisableTrialEnvironmentCreationByNonAdminUsers.IsUnknown() {
tenantSettingsDto.DisableTrialEnvironmentCreationByNonAdminUsers = tenantSettings.DisableTrialEnvironmentCreationByNonAdminUsers.ValueBoolPointer()
}
if !tenantSettings.DisableCapacityAllocationByEnvironmentAdmins.IsNull() && !tenantSettings.DisableCapacityAllocationByEnvironmentAdmins.IsUnknown() {
tenantSettingsDto.DisableCapacityAllocationByEnvironmentAdmins = tenantSettings.DisableCapacityAllocationByEnvironmentAdmins.ValueBoolPointer()
}
if !tenantSettings.DisableSupportTicketsVisibleByAllUsers.IsNull() && !tenantSettings.DisableSupportTicketsVisibleByAllUsers.IsUnknown() {
tenantSettingsDto.DisableSupportTicketsVisibleByAllUsers = tenantSettings.DisableSupportTicketsVisibleByAllUsers.ValueBoolPointer()
}
tenantSettingsDto.WalkMeOptOut = helpers.BoolPointer(tenantSettings.WalkMeOptOut)
tenantSettingsDto.DisableNPSCommentsReachout = helpers.BoolPointer(tenantSettings.DisableNPSCommentsReachout)
tenantSettingsDto.DisableNewsletterSendout = helpers.BoolPointer(tenantSettings.DisableNewsletterSendout)
tenantSettingsDto.DisableEnvironmentCreationByNonAdminUsers = helpers.BoolPointer(tenantSettings.DisableEnvironmentCreationByNonAdminUsers)
tenantSettingsDto.DisablePortalsCreationByNonAdminUsers = helpers.BoolPointer(tenantSettings.DisablePortalsCreationByNonAdminUsers)
tenantSettingsDto.DisableSurveyFeedback = helpers.BoolPointer(tenantSettings.DisableSurveyFeedback)
tenantSettingsDto.DisableTrialEnvironmentCreationByNonAdminUsers = helpers.BoolPointer(tenantSettings.DisableTrialEnvironmentCreationByNonAdminUsers)
tenantSettingsDto.DisableCapacityAllocationByEnvironmentAdmins = helpers.BoolPointer(tenantSettings.DisableCapacityAllocationByEnvironmentAdmins)
tenantSettingsDto.DisableSupportTicketsVisibleByAllUsers = helpers.BoolPointer(tenantSettings.DisableSupportTicketsVisibleByAllUsers)

if !tenantSettings.PowerPlatform.IsNull() && !tenantSettings.PowerPlatform.IsUnknown() {
powerPlatformAttributes := tenantSettings.PowerPlatform.Attributes()
Expand Down