Skip to content

Commit d83beea

Browse files
committed
fix(ci): reduce cyclomatic complexity in cloudstorage convertGCPRecommendation
Extract extractGCPResourceType, extractGCPSavings, and termYearsFromLabel helpers from convertGCPRecommendation in the cloudstorage package, mirroring the existing refactor in the cloudsql and memorystore packages. Complexity drops from 13 to under the limit, clearing the gocyclo pre-commit hook failure on CI. Behavior is unchanged.
1 parent 2ef3961 commit d83beea

1 file changed

Lines changed: 24 additions & 22 deletions

File tree

‎providers/gcp/services/cloudstorage/client.go‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -447,14 +447,13 @@ func skuMatchesStorageClass(sku *cloudbilling.Sku, storageClass, region string)
447447
return true
448448
}
449449

450-
// extractResourceTypeFromContent extracts the last path segment of the first
451-
// non-empty Operation.Resource across all operation groups. Used by all four
452-
// GCP service converters to set rec.ResourceType from Recommender payloads.
453-
func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent) string {
454-
if content == nil || content.OperationGroups == nil {
450+
// extractGCPResourceType returns the last path segment of the first non-empty
451+
// resource field found across all operation groups, or "" if none is present.
452+
func extractGCPResourceType(rec *recommenderpb.Recommendation) string {
453+
if rec.Content == nil || rec.Content.OperationGroups == nil {
455454
return ""
456455
}
457-
for _, opGroup := range content.OperationGroups {
456+
for _, opGroup := range rec.Content.OperationGroups {
458457
for _, op := range opGroup.Operations {
459458
if op.Resource == "" {
460459
continue
@@ -468,13 +467,13 @@ func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent
468467
return ""
469468
}
470469

471-
// extractEstimatedSavings returns the negative of the PrimaryImpact cost
472-
// projection (GCP encodes savings as a negative cost delta).
473-
func extractEstimatedSavings(gcpRec *recommenderpb.Recommendation) float64 {
474-
if gcpRec.PrimaryImpact == nil {
470+
// extractGCPSavings returns the estimated monthly savings (positive value)
471+
// from the primary cost impact of a GCP recommendation, or 0 if absent.
472+
func extractGCPSavings(rec *recommenderpb.Recommendation) float64 {
473+
if rec.PrimaryImpact == nil {
475474
return 0
476475
}
477-
costProj := gcpRec.PrimaryImpact.GetCostProjection()
476+
costProj := rec.PrimaryImpact.GetCostProjection()
478477
if costProj == nil || costProj.Cost == nil {
479478
return 0
480479
}
@@ -502,6 +501,15 @@ func (c *CloudStorageClient) fillStoragePricing(ctx context.Context, rec *common
502501
}
503502
}
504503

504+
// termYearsFromLabel converts a term string such as "1yr" or "3yr" to an
505+
// integer number of years (defaults to 1 for any unrecognised value).
506+
func termYearsFromLabel(term string) int {
507+
if term == "3yr" || term == "3" {
508+
return 3
509+
}
510+
return 1
511+
}
512+
505513
// convertGCPRecommendation converts a GCP Recommender recommendation to common format.
506514
// It also calls getStoragePricing to fill CommitmentCost/OnDemandCost/SavingsPercentage/
507515
// BreakEvenMonths so the scorer can filter and rank GCP recommendations correctly
@@ -517,6 +525,7 @@ func (c *CloudStorageClient) convertGCPRecommendation(ctx context.Context, gcpRe
517525
term = "1yr"
518526
}
519527

528+
520529
rec := &common.Recommendation{
521530
Provider: common.ProviderGCP,
522531
Service: common.ServiceStorage,
@@ -528,28 +537,21 @@ func (c *CloudStorageClient) convertGCPRecommendation(ctx context.Context, gcpRe
528537
PaymentOption: paymentOption,
529538
}
530539

531-
rec.ResourceType = extractResourceTypeFromContent(gcpRec.Content)
532-
rec.EstimatedSavings = extractEstimatedSavings(gcpRec)
540+
rec.ResourceType = extractGCPResourceType(gcpRec)
541+
rec.EstimatedSavings = extractGCPSavings(gcpRec)
533542

534543
// Thread pricing into the converter so the scorer can rank/filter GCP recs
535544
// correctly (issue #1022 C2).
536545
if rec.ResourceType != "" {
537-
termYears := 1
538-
if rec.Term == "3yr" || rec.Term == "3" {
539-
termYears = 3
540-
}
541-
c.fillStoragePricing(ctx, rec, termYears)
546+
c.fillStoragePricing(ctx, rec, termYearsFromLabel(rec.Term))
542547
}
543548

544549
// Populate RecurringMonthlyCost from the billing API. Cloud Storage
545550
// committed-use discounts are monthly-payment commitments, so the
546551
// per-month charge is CommitmentPrice / termMonths. A billing lookup
547552
// failure is non-fatal: log the warning and leave RecurringMonthlyCost
548553
// nil so the frontend renders "—" rather than a stale value.
549-
termYears := 1
550-
if rec.Term == "3yr" || rec.Term == "3" {
551-
termYears = 3
552-
}
554+
termYears := termYearsFromLabel(rec.Term)
553555
if pricing, err := c.getStoragePricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
554556
log.Printf("cloudstorage: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
555557
} else {

0 commit comments

Comments
 (0)