Fix customer subject validation (#2385)

Author: GAlexIHU

app/common/subscription.go

@@ -92,7 +92,7 @@ func NewSubscriptionServices(
 		Logger:              logger.With("subsystem", "subscription.change.service"),
 	})
 
-	validator, err := subscriptioncustomer.NewValidator(subscriptionService)
+	validator, err := subscriptioncustomer.NewValidator(subscriptionService, customerService)
 	if err != nil {
 		return SubscriptionServiceWithWorkflow{}, err
 	}

openmeter/subscription/service/service.go

@@ -57,7 +57,7 @@ func (s *service) Create(ctx context.Context, namespace string, spec subscriptio
 		return def, fmt.Errorf("spec is invalid: %w", err)
 	}
 
-	// Fetch the customer
+	// Fetch the customer & validate the customer
 	cust, err := s.CustomerService.GetCustomer(ctx, customer.GetCustomerInput{
 		Namespace: namespace,
 		ID:        spec.CustomerId,
@@ -70,9 +70,21 @@ func (s *service) Create(ctx context.Context, namespace string, spec subscriptio
 		return def, fmt.Errorf("customer is nil")
 	}
 
-	if cust.Currency != nil {
-		if string(*cust.Currency) != string(spec.Currency) {
-			return def, models.NewGenericValidationError(fmt.Errorf("currency mismatch: customer currency is %s, but subscription currency is %s", *cust.Currency, spec.Currency))
+	if _, err := cust.UsageAttribution.GetSubjectKey(); err != nil {
+		if spec.HasEntitlements() {
+			return def, models.NewGenericValidationError(errors.New("customer has no subject but subscription has entitlements"))
+		}
+
+		if spec.HasMeteredBillables() {
+			return def, models.NewGenericValidationError(errors.New("customer has no subject but subscription has metered billables"))
+		}
+	}
+
+	if spec.HasBillables() {
+		if cust.Currency != nil {
+			if string(*cust.Currency) != string(spec.Currency) {
+				return def, models.NewGenericValidationError(fmt.Errorf("currency mismatch: customer currency is %s, but subscription currency is %s", *cust.Currency, spec.Currency))
+			}
 		}
 	}
 
@@ -136,8 +148,8 @@ func (s *service) Create(ctx context.Context, namespace string, spec subscriptio
 			return def, err
 		}
 
-		// Let's set the customer's currency to the subscription currency (if not already set)
-		if cust.Currency == nil {
+		// Let's set the customer's currency to the subscription currency for paid subscriptions (if not already set)
+		if cust.Currency == nil && spec.HasBillables() {
 			if _, err := s.CustomerService.UpdateCustomer(ctx, customer.UpdateCustomerInput{
 				CustomerID: cust.GetID(),
 				CustomerMutate: customer.CustomerMutate{
@@ -194,6 +206,37 @@ func (s *service) Update(ctx context.Context, subscriptionID models.NamespacedID
 		return def, err
 	}
 
+	// Fetch the customer & validate the customer
+	cust, err := s.CustomerService.GetCustomer(ctx, customer.GetCustomerInput{
+		Namespace: view.Subscription.Namespace,
+		ID:        view.Subscription.CustomerId,
+	})
+	if err != nil {
+		return def, err
+	}
+
+	if cust == nil {
+		return def, fmt.Errorf("customer is nil")
+	}
+
+	if _, err := cust.UsageAttribution.GetSubjectKey(); err != nil {
+		if newSpec.HasEntitlements() {
+			return def, models.NewGenericValidationError(errors.New("customer has no subject but subscription has entitlements"))
+		}
+
+		if newSpec.HasMeteredBillables() {
+			return def, models.NewGenericValidationError(errors.New("customer has no subject but subscription has metered billables"))
+		}
+	}
+
+	if newSpec.HasBillables() {
+		if cust.Currency != nil {
+			if string(*cust.Currency) != string(newSpec.Currency) {
+				return def, models.NewGenericValidationError(fmt.Errorf("currency mismatch: customer currency is %s, but subscription currency is %s", *cust.Currency, newSpec.Currency))
+			}
+		}
+	}
+
 	return transaction.Run(ctx, s.TransactionManager, func(ctx context.Context) (subscription.Subscription, error) {
 		subs, err := s.sync(ctx, view, newSpec)
 		if err != nil {

openmeter/subscription/subscriptionspec.go

@@ -151,6 +151,24 @@ func (s *SubscriptionSpec) GetCurrentPhaseAt(t time.Time) (*SubscriptionPhaseSpe
 	return current, true
 }
 
+func (s *SubscriptionSpec) HasEntitlements() bool {
+	return lo.SomeBy(lo.Values(s.Phases), func(p *SubscriptionPhaseSpec) bool {
+		return p.HasEntitlements()
+	})
+}
+
+func (s *SubscriptionSpec) HasBillables() bool {
+	return lo.SomeBy(lo.Values(s.Phases), func(p *SubscriptionPhaseSpec) bool {
+		return p.HasBillables()
+	})
+}
+
+func (s *SubscriptionSpec) HasMeteredBillables() bool {
+	return lo.SomeBy(lo.Values(s.Phases), func(p *SubscriptionPhaseSpec) bool {
+		return p.HasMeteredBillables()
+	})
+}
+
 // For a phase in an Aligned subscription, there's a single aligned BillingPeriod for all items in that phase.
 // The period starts with the phase and iterates every BillingCadence duration, but can be reanchored to the time of an edit.
 func (s *SubscriptionSpec) GetAlignedBillingPeriodAt(phaseKey string, at time.Time) (timeutil.Period, error) {
@@ -370,6 +388,18 @@ func (s SubscriptionPhaseSpec) GetBillableItemsByKey() map[string][]*Subscriptio
 	return res
 }
 
+func (s SubscriptionPhaseSpec) HasEntitlements() bool {
+	return lo.SomeBy(lo.Flatten(lo.Values(s.ItemsByKey)), func(item *SubscriptionItemSpec) bool {
+		return item.RateCard.EntitlementTemplate != nil
+	})
+}
+
+func (s SubscriptionPhaseSpec) HasMeteredBillables() bool {
+	return lo.SomeBy(lo.Flatten(lo.Values(s.ItemsByKey)), func(item *SubscriptionItemSpec) bool {
+		return item.RateCard.Price != nil && item.RateCard.Price.Type() != productcatalog.FlatPriceType
+	})
+}
+
 func (s SubscriptionPhaseSpec) HasBillables() bool {
 	return len(s.GetBillableItemsByKey()) > 0
 }

openmeter/subscription/validators/customer/validator.go

@@ -13,19 +13,68 @@ import (
 
 var _ customer.RequestValidator = (*Validator)(nil)
 
-func NewValidator(subscriptionService subscription.Service) (*Validator, error) {
+func NewValidator(subscriptionService subscription.Service, customerService customer.Service) (*Validator, error) {
 	if subscriptionService == nil {
 		return nil, fmt.Errorf("subscription service is required")
 	}
+	if customerService == nil {
+		return nil, fmt.Errorf("customer service is required")
+	}
 
 	return &Validator{
 		subscriptionService: subscriptionService,
+		customerService:     customerService,
 	}, nil
 }
 
 type Validator struct {
 	customer.NoopRequestValidator
 	subscriptionService subscription.Service
+	customerService     customer.Service
+}
+
+func (v *Validator) ValidateUpdateCustomer(ctx context.Context, input customer.UpdateCustomerInput) error {
+	if err := input.Validate(); err != nil {
+		return err
+	}
+
+	// The subject association can only be changed if the customer doesn't have a subscription
+	subscriptions, err := v.subscriptionService.List(ctx, subscription.ListSubscriptionsInput{
+		Namespaces: []string{input.CustomerID.Namespace},
+		Customers:  []string{input.CustomerID.ID},
+		ActiveAt:   lo.ToPtr(clock.Now()),
+	})
+	if err != nil {
+		return err
+	}
+
+	hasSub := len(subscriptions.Items) > 0
+
+	// If there's an update to the subject keys we need additional checks
+	if input.CustomerMutate.UsageAttribution.SubjectKeys != nil {
+		currentCustomer, err := v.customerService.GetCustomer(ctx, customer.GetCustomerInput{
+			Namespace: input.CustomerID.Namespace,
+			ID:        input.CustomerID.ID,
+		})
+		if err != nil {
+			return err
+		}
+
+		// Let's check the two subjectKey arrays are the same
+		if hasSub {
+			if len(currentCustomer.UsageAttribution.SubjectKeys) != len(input.CustomerMutate.UsageAttribution.SubjectKeys) {
+				return fmt.Errorf("cannot change subject keys for customer with active subscriptions")
+			}
+
+			for i, key := range currentCustomer.UsageAttribution.SubjectKeys {
+				if key != input.CustomerMutate.UsageAttribution.SubjectKeys[i] {
+					return fmt.Errorf("cannot change subject keys for customer with active subscriptions")
+				}
+			}
+		}
+	}
+
+	return nil
 }
 
 func (v *Validator) ValidateDeleteCustomer(ctx context.Context, input customer.DeleteCustomerInput) error {