refactor: extend blocking everywhere where we serialize rate cards

Author: rolosp

openmeter/productcatalog/addon/adapter/adapter_test.go

@@ -450,6 +450,78 @@ func TestPostgresAdapter(t *testing.T) {
 	})
 }
 
+func TestListAddonsExcludeUnitConfig(t *testing.T) {
+	ctx := context.Background()
+
+	env := pctestutils.NewTestEnv(t)
+	t.Cleanup(func() { env.Close(t) })
+	env.DBSchemaMigrate(t)
+
+	namespace := pctestutils.NewTestNamespace(t)
+
+	require.NoError(t, env.Meter.ReplaceMeters(ctx, pctestutils.NewTestMeters(t, namespace)),
+		"replacing meters must not fail")
+	meters, err := env.Meter.ListMeters(ctx, meter.ListMetersParams{
+		Page:      pagination.Page{PageSize: 1000, PageNumber: 1},
+		Namespace: namespace,
+	})
+	require.NoError(t, err, "listing meters must not fail")
+	require.NotEmpty(t, meters.Items, "list of meters must not be empty")
+
+	feat, err := env.Feature.CreateFeature(ctx, pctestutils.NewTestFeatureFromMeter(t, &meters.Items[0]))
+	require.NoError(t, err, "creating feature must not fail")
+
+	// Plain add-on: flat rate card, no unit_config → v1-representable.
+	plainInput := pctestutils.NewTestAddon(t, namespace, &productcatalog.FlatFeeRateCard{
+		RateCardMeta: productcatalog.RateCardMeta{Key: "flat", Name: "Flat"},
+	})
+	plainInput.Addon.Key = "plain"
+	_, err = env.AddonRepository.CreateAddon(ctx, plainInput)
+	require.NoError(t, err, "creating plain add-on must not fail")
+
+	// unit_config add-on: usage-based rate card carrying a unit_config → not v1-representable.
+	ucInput := pctestutils.NewTestAddon(t, namespace, &productcatalog.UsageBasedRateCard{
+		RateCardMeta: productcatalog.RateCardMeta{
+			Key:        feat.Key,
+			Name:       "UC RateCard",
+			FeatureKey: lo.ToPtr(feat.Key),
+			FeatureID:  lo.ToPtr(feat.ID),
+			Price:      productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: decimal.NewFromInt(1)}),
+			UnitConfig: &productcatalog.UnitConfig{
+				Operation:        productcatalog.UnitConfigOperationDivide,
+				ConversionFactor: decimal.NewFromInt(1000),
+			},
+		},
+		BillingCadence: MonthPeriod,
+	})
+	ucInput.Addon.Key = "with-uc"
+	_, err = env.AddonRepository.CreateAddon(ctx, ucInput)
+	require.NoError(t, err, "creating unit_config add-on must not fail")
+
+	t.Run("included when ExcludeUnitConfig is false", func(t *testing.T) {
+		list, err := env.AddonRepository.ListAddons(ctx, addon.ListAddonsInput{
+			Namespaces: []string{namespace},
+		})
+		require.NoError(t, err, "listing add-ons must not fail")
+
+		keys := lo.Map(list.Items, func(a addon.Addon, _ int) string { return a.Key })
+		require.ElementsMatch(t, []string{"plain", "with-uc"}, keys)
+		require.Equal(t, 2, list.TotalCount, "TotalCount must count both add-ons")
+	})
+
+	t.Run("excluded when ExcludeUnitConfig is true, TotalCount stays consistent", func(t *testing.T) {
+		list, err := env.AddonRepository.ListAddons(ctx, addon.ListAddonsInput{
+			Namespaces:        []string{namespace},
+			ExcludeUnitConfig: true,
+		})
+		require.NoError(t, err, "listing add-ons must not fail")
+
+		keys := lo.Map(list.Items, func(a addon.Addon, _ int) string { return a.Key })
+		require.ElementsMatch(t, []string{"plain"}, keys)
+		require.Equal(t, 1, list.TotalCount, "TotalCount must exclude the unit_config add-on, not just the page slice")
+	})
+}
+
 // TestFromPlanRateCardRowMapsUnitConfig guards the cross-package mapper used when an
 // add-on is loaded with expanded linked plans
 // (FromAddonRow → FromPlanAddonRow → FromPlanRow → FromPlanPhaseRow → FromPlanRateCardRow).

openmeter/productcatalog/addon/adapter/addon.go

@@ -50,6 +50,13 @@ func (a *adapter) ListAddons(ctx context.Context, params addon.ListAddonsInput)
 		query = filter.ApplyToQuery(query, params.Name, addondb.FieldName)
 		query = filter.ApplyToQuery(query, params.Currency, addondb.FieldCurrency)
 
+		if params.ExcludeUnitConfig {
+			query = query.Where(addondb.Not(addondb.HasRatecardsWith(
+				addonratecarddb.UnitConfigNotNil(),
+				addonratecarddb.DeletedAtIsNil(),
+			)))
+		}
+
 		if !params.IncludeDeleted {
 			query = query.Where(addondb.DeletedAtIsNil())
 		}

openmeter/productcatalog/addon/httpdriver/addon.go

@@ -56,6 +56,8 @@ func (h *handler) ListAddons() ListAddonsHandler {
 				KeyVersions:    lo.FromPtr(params.KeyVersion),
 				IncludeDeleted: lo.FromPtr(params.IncludeDeleted),
 				Status:         statusFilter,
+				// The v1 API cannot represent unit_config; exclude such add-ons from the v1 list at the query layer.
+				ExcludeUnitConfig: true,
 			}
 
 			if params.Id != nil {

openmeter/productcatalog/addon/service.go

@@ -95,6 +95,9 @@ type ListAddonsInput struct {
 	Key      *filter.FilterString
 	Name     *filter.FilterString
 	Currency *filter.FilterString
+
+	// ExcludeUnitConfig omits add-ons carrying a unit_config conversion on any of their rate cards.
+	ExcludeUnitConfig bool
 }
 
 func (i ListAddonsInput) Validate() error {

openmeter/productcatalog/http/mapping.go

@@ -22,6 +22,15 @@ func FromRateCard(r productcatalog.RateCard) (api.RateCard, error) {
 
 	meta := r.AsMeta()
 
+	// The v1 rate card types have no unit_config field. This mapper is the single serialization choke
+	// point for every v1 surface that emits a rate card, so it fails closed here rather than silently
+	// dropping the conversion and misrepresenting billing: reject the object, available via the v3 API.
+	// List surfaces exclude unit_config objects at the query layer so they never reach this mapper;
+	// subscription items are serialized without this mapper, so subscription GET guards them via Spec.HasUnitConfig.
+	if meta.UnitConfig != nil {
+		return resp, productcatalog.ErrUnitConfigNotRepresentable
+	}
+
 	var featureKey *string
 	if meta.FeatureKey != nil {
 		featureKey = meta.FeatureKey
@@ -923,7 +932,7 @@ func FromAnnotations(annotations models.Annotations) *api.Annotations {
 		return nil
 	}
 
-	return lo.ToPtr((api.Annotations)(annotations))
+	return lo.ToPtr(api.Annotations(annotations))
 }
 
 func FromValidationErrors(issues models.ValidationIssues) *[]api.ValidationError {

openmeter/productcatalog/http/mapping_test.go

@@ -0,0 +1,59 @@
+package http
+
+import (
+	"testing"
+
+	decimal "github.com/alpacahq/alpacadecimal"
+	"github.com/samber/lo"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/openmeterio/openmeter/openmeter/productcatalog"
+	"github.com/openmeterio/openmeter/pkg/datetime"
+	"github.com/openmeterio/openmeter/pkg/models"
+)
+
+// FromRateCard is the single v1 serialization choke point for rate cards. It must fail closed on a
+// unit_config-bearing card (the v1 types cannot represent it) so no v1 surface silently strips the
+// conversion, while a plain card still serializes normally.
+func TestFromRateCardRejectsUnitConfig(t *testing.T) {
+	cadence := datetime.NewISODuration(0, 1, 0, 0, 0, 0, 0)
+
+	t.Run("unit_config card is rejected with the typed code", func(t *testing.T) {
+		rc := &productcatalog.UsageBasedRateCard{
+			RateCardMeta: productcatalog.RateCardMeta{
+				Key:        "feat-1",
+				Name:       "Feature 1",
+				FeatureKey: lo.ToPtr("feat-1"),
+				Price:      productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: decimal.NewFromInt(1)}),
+				UnitConfig: &productcatalog.UnitConfig{
+					Operation:        productcatalog.UnitConfigOperationDivide,
+					ConversionFactor: decimal.NewFromInt(1000),
+				},
+			},
+			BillingCadence: cadence,
+		}
+
+		_, err := FromRateCard(rc)
+		require.Error(t, err)
+
+		issues, convErr := models.AsValidationIssues(err)
+		require.NoError(t, convErr)
+		require.Len(t, issues, 1)
+		assert.Equal(t, productcatalog.ErrCodeUnitConfigNotRepresentable, issues[0].Code())
+	})
+
+	t.Run("plain card serializes without error", func(t *testing.T) {
+		rc := &productcatalog.FlatFeeRateCard{
+			RateCardMeta: productcatalog.RateCardMeta{
+				Key:   "flat-1",
+				Name:  "Flat 1",
+				Price: productcatalog.NewPriceFrom(productcatalog.FlatPrice{Amount: decimal.NewFromInt(10), PaymentTerm: productcatalog.InArrearsPaymentTerm}),
+			},
+			BillingCadence: &cadence,
+		}
+
+		_, err := FromRateCard(rc)
+		require.NoError(t, err)
+	})
+}