From eade22e4c54d7f87f8f961199e20895c6f00a33d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 6 Nov 2026 07:27:50 +0100 Subject: [PATCH 1/2] feat(common)!: add PurchaseResult.OfferingClass for adopted re-drives On an adopted re-drive the field carries the offering class AWS reports for the existing commitment, so a re-drive that adopted an RI of another class isn't mislabeled downstream. The EC2 provider sets it in a follow-up change, after the providers re-pin pkg. The Cost doc now states what each provider reports on adoption today. BREAKING CHANGE: unkeyed composite literals of common.PurchaseResult no longer compile; keyed literals are unaffected. Refs #211 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +++++ pkg/common/types.go | 19 +++++++++++++++---- pkg/common/types_test.go | 13 +++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bcb2207..158b7e6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -278,6 +278,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). longer compiles; keyed literals are unaffected. The field is true when a provider adopted a commitment that already existed for the same idempotency token (#211) +- **Breaking (source):** `common.PurchaseResult` gained an `OfferingClass` field. + Downstream code that builds it with unkeyed composite literals no longer + compiles; keyed literals are unaffected. On an adopted re-drive it carries the + offering class AWS reports for the existing RI (the EC2 provider sets it in a + follow-up change); empty otherwise (#211). ### Fixed diff --git a/pkg/common/types.go b/pkg/common/types.go index a82eb104..51af271c 100644 --- a/pkg/common/types.go +++ b/pkg/common/types.go @@ -390,10 +390,21 @@ type PurchaseResult struct { Error error `json:"error,omitempty"` // Cost is the total upfront charge for the whole purchase (unit price x count). - // Nil means unknown; a non-nil zero means no upfront charge. + // Nil means unknown; a non-nil zero means no upfront charge. On an adopted + // re-drive (ExistingCommitment) the original purchase's charge is not known: + // the AWS RI services (EC2, RDS, ElastiCache, MemoryDB, OpenSearch, Redshift) + // return nil. GCP Compute Engine reports 0 today and returns nil after a + // follow-up change for #211. Azure reports 0 for a monthly billing plan and + // nil for an upfront one, as on a fresh purchase. Cost *float64 `json:"cost"` - CommitmentID string `json:"commitment_id,omitempty"` + CommitmentID string `json:"commitment_id,omitempty"` + + // OfferingClass is, on an adopted re-drive, the offering class AWS reports + // for the existing commitment; empty otherwise. The EC2 provider sets it in + // a follow-up change for #211. + OfferingClass string `json:"offering_class,omitempty"` + Recommendation Recommendation `json:"recommendation"` Success bool `json:"success"` DryRun bool `json:"dry_run"` @@ -401,8 +412,8 @@ type PurchaseResult struct { // ExistingCommitment is true when the provider adopted a commitment that // already existed for the same idempotency token (a re-drive of an earlier // attempt) instead of creating a new one. Success stays true and - // CommitmentID names the adopted commitment. Cost semantics on adoption are - // unspecified until the provider changes land. False on a fresh purchase. + // CommitmentID names the adopted commitment. See Cost for its value on + // adoption. False on a fresh purchase. ExistingCommitment bool `json:"existing_commitment,omitempty"` } diff --git a/pkg/common/types_test.go b/pkg/common/types_test.go index 66246d1b..0a2897b2 100644 --- a/pkg/common/types_test.go +++ b/pkg/common/types_test.go @@ -45,6 +45,19 @@ func TestPurchaseResult_ExistingCommitmentJSON(t *testing.T) { assert.True(t, decoded.ExistingCommitment) } +func TestPurchaseResult_OfferingClassJSON(t *testing.T) { + empty, err := json.Marshal(PurchaseResult{Success: true, CommitmentID: "ri-1"}) + require.NoError(t, err) + assert.NotContains(t, string(empty), "offering_class") + + set, err := json.Marshal(PurchaseResult{Success: true, CommitmentID: "ri-1", ExistingCommitment: true, OfferingClass: "convertible"}) + require.NoError(t, err) + assert.Contains(t, string(set), `"offering_class":"convertible"`) + var decoded PurchaseResult + require.NoError(t, json.Unmarshal(set, &decoded)) + assert.Equal(t, "convertible", decoded.OfferingClass) +} + func TestRecommendation_CommitmentCurrencyJSONAndScaling(t *testing.T) { t.Parallel() for _, tt := range []struct { From 4c7645c0f6d911bb86d930700ba73bc339565ef7 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 6 Nov 2026 19:58:25 +0100 Subject: [PATCH 2/2] fix(common): decode a Recommendation's details object from JSON encoding/json cannot decode an object into the ServiceDetails interface, so a Recommendation, or a PurchaseResult carrying one, failed to decode whenever details held an object. Recommendation.UnmarshalJSON now decodes details through DecodeServiceDetailsFor for the recommendation's service. As with encoding/json, absent details leave the field unchanged and null sets it nil. Details for a service with no typed details fail with ErrUntypedServiceDetails. Refs #211 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 +++ pkg/common/service_details_codec.go | 29 +++++++++++ pkg/common/service_details_codec_test.go | 63 ++++++++++++++++++++++++ 3 files changed, 100 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 158b7e6e..ba3d84f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -286,6 +286,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed +- **`common`:** a `Recommendation`, and a `PurchaseResult` carrying one (with + no `Error`), now decodes from JSON when `details` holds an object. `details` + is decoded into the typed pointer for the recommendation's service through + `DecodeServiceDetailsFor`; absent `details` leave the field unchanged and + null sets it nil, as encoding/json does, and details for a service with no + typed details fail with `ErrUntypedServiceDetails`. Before this, + encoding/json could not decode an object into the `ServiceDetails` interface + (#211). - **`aws` RI recommendations:** a fractional Cost Explorer quantity now rounds down instead of to the nearest integer, so a purchase never exceeds CE's quantity. The upfront, recurring monthly cost, monthly savings and savings diff --git a/pkg/common/service_details_codec.go b/pkg/common/service_details_codec.go index bffef24f..b2f83c48 100644 --- a/pkg/common/service_details_codec.go +++ b/pkg/common/service_details_codec.go @@ -172,3 +172,32 @@ func newDetailsForService(service string) (ServiceDetails, bool) { } return nil, false } + +// UnmarshalJSON decodes details into the typed pointer for the +// recommendation's service (DecodeServiceDetailsFor), so a Recommendation, and +// a PurchaseResult carrying one, round-trips through JSON. encoding/json cannot +// decode an object into the ServiceDetails interface on its own. As with +// encoding/json, absent details leave Details unchanged and null sets it nil. +func (r *Recommendation) UnmarshalJSON(data []byte) error { + type plain Recommendation + aux := struct { + *plain + Details json.RawMessage `json:"details,omitempty"` + }{plain: (*plain)(r)} + if err := json.Unmarshal(data, &aux); err != nil { + return err + } + if len(aux.Details) == 0 { + return nil + } + if bytes.Equal(aux.Details, jsonNullBytes) { + r.Details = nil + return nil + } + details, err := DecodeServiceDetailsFor(string(r.Service), aux.Details) + if err != nil { + return err + } + r.Details = details + return nil +} diff --git a/pkg/common/service_details_codec_test.go b/pkg/common/service_details_codec_test.go index 7786091d..f428855a 100644 --- a/pkg/common/service_details_codec_test.go +++ b/pkg/common/service_details_codec_test.go @@ -310,3 +310,66 @@ func TestDecodeServiceDetailsFor_EveryEmittedPair(t *testing.T) { }) } } + +func TestRecommendation_DetailsJSONRoundTrip(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + name string + rec Recommendation + details ServiceDetails + }{ + {"ec2 compute", Recommendation{Service: ServiceEC2, Details: &ComputeDetails{InstanceType: "m5.large", Platform: "Linux/UNIX", Tenancy: "default", Scope: "Region"}}, &ComputeDetails{InstanceType: "m5.large", Platform: "Linux/UNIX", Tenancy: "default", Scope: "Region"}}, + {"value compute decodes to a pointer", Recommendation{Service: ServiceCompute, Details: ComputeDetails{InstanceType: "n2-standard-4"}}, &ComputeDetails{InstanceType: "n2-standard-4"}}, + {"rds database", Recommendation{Service: ServiceRDS, Details: &DatabaseDetails{Engine: "postgres", AZConfig: "multi-az", InstanceClass: "db.r6g.large"}}, &DatabaseDetails{Engine: "postgres", AZConfig: "multi-az", InstanceClass: "db.r6g.large"}}, + {"no details stays nil", Recommendation{Service: ServiceEC2}, nil}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + payload, err := json.Marshal(tt.rec) + require.NoError(t, err) + var got Recommendation + require.NoError(t, json.Unmarshal(payload, &got)) + assert.Equal(t, tt.details, got.Details) + assert.Equal(t, tt.rec.Service, got.Service) + }) + } + + t.Run("explicit null details stays nil", func(t *testing.T) { + t.Parallel() + var got Recommendation + require.NoError(t, json.Unmarshal([]byte(`{"service":"ec2","details":null}`), &got)) + assert.Nil(t, got.Details) + }) + + t.Run("decoding into a filled recommendation follows encoding/json", func(t *testing.T) { + t.Parallel() + prior := &ComputeDetails{InstanceType: "m5.large"} + absent := Recommendation{Service: ServiceEC2, Details: prior} + require.NoError(t, json.Unmarshal([]byte(`{"count":2}`), &absent)) + assert.Same(t, prior, absent.Details, "absent details leave Details unchanged") + null := Recommendation{Service: ServiceEC2, Details: prior} + require.NoError(t, json.Unmarshal([]byte(`{"details":null}`), &null)) + assert.Nil(t, null.Details, "null details set Details nil") + }) + + t.Run("details for a service with no typed details fail", func(t *testing.T) { + t.Parallel() + var got Recommendation + err := json.Unmarshal([]byte(`{"service":"no-such-service","details":{"x":1}}`), &got) + require.ErrorIs(t, err, ErrUntypedServiceDetails) + }) + + t.Run("purchase result with details keeps its offering class", func(t *testing.T) { + t.Parallel() + in := PurchaseResult{ + Success: true, CommitmentID: "ri-1", ExistingCommitment: true, OfferingClass: "convertible", + Recommendation: Recommendation{Service: ServiceEC2, Details: &ComputeDetails{InstanceType: "m5.large"}}, + } + payload, err := json.Marshal(in) + require.NoError(t, err) + var got PurchaseResult + require.NoError(t, json.Unmarshal(payload, &got)) + assert.Equal(t, "convertible", got.OfferingClass) + assert.Equal(t, &ComputeDetails{InstanceType: "m5.large"}, got.Recommendation.Details) + }) +}