Skip to content

Commit

Permalink
refactor(subs): restructure patches
Browse files Browse the repository at this point in the history
  • Loading branch information
GAlexIHU committed Jan 17, 2025
1 parent a4f7ca2 commit bb3665b
Show file tree
Hide file tree
Showing 8 changed files with 157 additions and 111 deletions.
6 changes: 6 additions & 0 deletions api/spec/src/productcatalog/subscription.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,9 @@ model PlanSubscriptionChange {
description?: string;
}

/**
* Edit operations for a subscription.
*/
@friendlyName("SubscriptionEditOperation")
@oneOf
@discriminator("op")
Expand All @@ -344,6 +347,9 @@ union SubscriptionEditOperation {
EditSubscriptionStretchPhase,
}

/**
* Edit operation enum.
*/
@friendlyName("EditOp")
enum EditOp {
AddItem: "add_item",
Expand Down
68 changes: 18 additions & 50 deletions openmeter/subscription/patch/additem.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"

"github.com/openmeterio/openmeter/openmeter/subscription"
"github.com/openmeterio/openmeter/pkg/datex"
)

type PatchAddItem struct {
Expand Down Expand Up @@ -44,59 +43,28 @@ func (a PatchAddItem) Validate() error {
var _ subscription.ValuePatch[subscription.SubscriptionItemSpec] = PatchAddItem{}

func (a PatchAddItem) ApplyTo(spec *subscription.SubscriptionSpec, actx subscription.ApplyContext) error {
phase, ok := spec.Phases[a.PhaseKey]
if !ok {
return &subscription.PatchValidationError{Msg: fmt.Sprintf("phase %s not found", a.PhaseKey)}
phase, rel, err := phaseContentHelper{spec: *spec, actx: actx}.GetPhaseForEdit(a.PhaseKey)
if err != nil {
return err
}

phaseStartTime, _ := phase.StartAfter.AddTo(spec.ActiveFrom)

// Checks we need:

// 1. You cannot add items to previous phases
currentPhase, exists := spec.GetCurrentPhaseAt(actx.CurrentTime)
if !exists {
// If the current phase doesn't exist then either all phases are in the past or in the future
// If all phases are in the past then no addition is possible
// If all phases are in the past then the selected one is also in the past
if st, _ := phase.StartAfter.AddTo(spec.ActiveFrom); st.Before(actx.CurrentTime) {
return &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot add item to phase %s which starts before current phase", a.PhaseKey)}
} else {
// If it's added to a future phase, the matching key for the phase has to be empty
if len(phase.ItemsByKey) > 0 {
return &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot add item to future phase %s which already has items", a.PhaseKey)}
}
}
} else {
currentPhaseStartTime, _ := currentPhase.StartAfter.AddTo(spec.ActiveFrom)

// If the selected phase is before the current phase, it's forbidden
if phaseStartTime.Before(currentPhaseStartTime) {
return &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot add item to phase %s which starts before current phase", a.PhaseKey)}
} else if phase.PhaseKey == currentPhase.PhaseKey {
// Sanity check
if actx.CurrentTime.Before(phaseStartTime) {
return fmt.Errorf("Current time is before the current phase start which is impossible")
}
cadenceHelper := relativeCadenceHelper{
contentType: "item",
phaseStartTime: phaseStartTime,
phaseKey: a.PhaseKey,
rel: rel,
actx: actx,
}
if err := cadenceHelper.ValidateRelativeCadence(&a.CreateInput.CadenceOverrideRelativeToPhaseStart); err != nil {
return err
}

// 2. If it's added to the current phase, the specified start time cannot point to the past
if a.CreateInput.CadenceOverrideRelativeToPhaseStart.ActiveFromOverride != nil {
iST, _ := a.CreateInput.CadenceOverrideRelativeToPhaseStart.ActiveFromOverride.AddTo(phaseStartTime)
if iST.Before(actx.CurrentTime) {
return &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot add item to phase %s which would become active in the past at %s", a.PhaseKey, iST)}
}
} else {
// 3. If it's added to the current phase, and start time is not specified, it will be set for the current time, as you cannot change the past
diff := datex.Between(phaseStartTime, actx.CurrentTime)
a.CreateInput.CadenceOverrideRelativeToPhaseStart.ActiveFromOverride = &diff
}
} else if phaseStartTime.After(currentPhaseStartTime) {
// 4. If you're adding it to a future phase, the matching key for the phase has to be empty
if len(phase.ItemsByKey[a.ItemKey]) > 0 {
return &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot add item to future phase %s which already has items", a.PhaseKey)}
}
} else {
return fmt.Errorf("didn't enter any logical branch")
// If you're adding it to a future phase, the matching key for the phase has to be empty
if rel == isFuturePhase {
if len(phase.ItemsByKey[a.ItemKey]) > 0 {
return &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot add item to future phase %s which already has items", a.PhaseKey)}
}
}

Expand All @@ -109,7 +77,7 @@ func (a PatchAddItem) ApplyTo(spec *subscription.SubscriptionSpec, actx subscrip
// If it's added to the current phase, we need to close the activity of any current item if present
hasCurrentItemAndShouldCloseCurrentItemForKey := false

if exists && currentPhase.PhaseKey == phase.PhaseKey {
if rel == isCurrentPhase {
if len(phase.ItemsByKey[a.ItemKey]) > 0 {
hasCurrentItemAndShouldCloseCurrentItemForKey = true
}
Expand Down
6 changes: 3 additions & 3 deletions openmeter/subscription/patch/additem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestAddItem(t *testing.T) {
Ctx: subscription.ApplyContext{
CurrentTime: now,
},
ExpectedError: &subscription.PatchValidationError{Msg: "phase invalid_phase not found"},
ExpectedError: &subscription.PatchConflictError{Msg: "phase invalid_phase not found"},
},
{
Name: "Cannot add item to previous phase",
Expand All @@ -66,7 +66,7 @@ func TestAddItem(t *testing.T) {
// We're doing this edit during the 2nd phase
CurrentTime: now.AddDate(0, 1, 2),
},
ExpectedError: &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot add item to phase %s which starts before current phase", p.Phases[0].Key)},
ExpectedError: &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot change contents of phase %s which starts before current phase", p.Phases[0].Key)},
},
{
Name: "Cannot add item to old subscription (where everything is in the past)",
Expand All @@ -85,7 +85,7 @@ func TestAddItem(t *testing.T) {
Ctx: subscription.ApplyContext{
CurrentTime: now.AddDate(1, 0, 0),
},
ExpectedError: &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot add item to phase %s which starts before current phase", p.Phases[0].Key)},
ExpectedError: &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot change contents of phase %s which starts before current phase", p.Phases[0].Key)},
},
{
Name: "Cannot add item to current phase which would become active in the past",
Expand Down
26 changes: 4 additions & 22 deletions openmeter/subscription/patch/removeitem.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ func (r PatchRemoveItem) Validate() error {
var _ subscription.Patch = PatchRemoveItem{}

func (r PatchRemoveItem) ApplyTo(spec *subscription.SubscriptionSpec, actx subscription.ApplyContext) error {
phase, ok := spec.Phases[r.PhaseKey]
if !ok {
return &subscription.PatchValidationError{Msg: fmt.Sprintf("phase %s not found", r.PhaseKey)}
phase, rel, err := phaseContentHelper{spec: *spec, actx: actx}.GetPhaseForEdit(r.PhaseKey)
if err != nil {
return err
}

phaseStartTime, _ := phase.StartAfter.AddTo(spec.ActiveFrom)
Expand All @@ -46,26 +46,8 @@ func (r PatchRemoveItem) ApplyTo(spec *subscription.SubscriptionSpec, actx subsc
return &subscription.PatchValidationError{Msg: fmt.Sprintf("items for key %s doesn't exists in phase %s", r.ItemKey, r.PhaseKey)}
}

// Checks we need:
// 1. You cannot remove items from previous phases
currentPhase, exists := spec.GetCurrentPhaseAt(actx.CurrentTime)
if !exists {
// either all phases are in the past or in the future
// if all phases are in the past then no removal is possible
//
// If all phases are in the past then the selected one is also in the past
if st, _ := phase.StartAfter.AddTo(spec.ActiveFrom); st.Before(actx.CurrentTime) {
return &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot remove item from phase %s which starts before current phase", r.PhaseKey)}
}
} else {
currentPhaseStartTime, _ := currentPhase.StartAfter.AddTo(spec.ActiveFrom)
if phaseStartTime.Before(currentPhaseStartTime) {
return &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot remove item from phase %s which starts before current phase", r.PhaseKey)}
}
}

// Finally, lets try to remove the item
if exists && currentPhase.PhaseKey == r.PhaseKey {
if rel == isCurrentPhase {
// If it's removed from the current phase, we should set its end time to the current time, instead of deleting it (as we cannot falsify history)

diff := datex.Between(phaseStartTime, actx.CurrentTime)
Expand Down
6 changes: 3 additions & 3 deletions openmeter/subscription/patch/removeitem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestRemoveItem(t *testing.T) {
},
GetSpec: getSpec,
Ctx: subscription.ApplyContext{CurrentTime: now},
ExpectedError: &subscription.PatchValidationError{
ExpectedError: &subscription.PatchConflictError{
Msg: "phase notfound not found",
},
},
Expand Down Expand Up @@ -102,7 +102,7 @@ func TestRemoveItem(t *testing.T) {
CurrentTime: now.AddDate(0, 1, 1), // same as ts above
},
ExpectedError: &subscription.PatchForbiddenError{
Msg: "cannot remove item from phase test_phase_1 which starts before current phase",
Msg: "cannot change contents of phase test_phase_1 which starts before current phase",
},
},
{
Expand All @@ -122,7 +122,7 @@ func TestRemoveItem(t *testing.T) {
CurrentTime: now.AddDate(1, 0, 0), // We're far in the future
},
ExpectedError: &subscription.PatchForbiddenError{
Msg: "cannot remove item from phase test_phase_1 which starts before current phase",
Msg: "cannot change contents of phase test_phase_1 which starts before current phase",
},
},
{
Expand Down
83 changes: 83 additions & 0 deletions openmeter/subscription/patch/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package patch

import (
"fmt"
"time"

"github.com/openmeterio/openmeter/openmeter/subscription"
"github.com/openmeterio/openmeter/pkg/datex"
)

type timeRelation int

const (
isCurrentPhase timeRelation = iota
isFuturePhase
)

// Helper utility to get a given phase and determine its relation to the current phase
type phaseContentHelper struct {
spec subscription.SubscriptionSpec
actx subscription.ApplyContext
}

func (p phaseContentHelper) GetPhaseForEdit(phaseKey string) (*subscription.SubscriptionPhaseSpec, timeRelation, error) {
phase, ok := p.spec.Phases[phaseKey]
if !ok {
return nil, 0, &subscription.PatchConflictError{Msg: fmt.Sprintf("phase %s not found", phaseKey)}
}

phaseStartTime, _ := phase.StartAfter.AddTo(p.spec.ActiveFrom)

// 1. You cannot add items to previous phases
currentPhase, exists := p.spec.GetCurrentPhaseAt(p.actx.CurrentTime)
if !exists {
// If the current phase doesn't exist then either all phases are in the past or in the future
// If all phases are in the past then no addition is possible
// If all phases are in the past then the selected one is also in the past
if st, _ := phase.StartAfter.AddTo(p.spec.ActiveFrom); st.Before(p.actx.CurrentTime) {
return nil, 0, &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot change contents of phase %s which starts before current phase", phaseKey)}
} else {
return phase, isFuturePhase, nil
}
} else {
currentPhaseStartTime, _ := currentPhase.StartAfter.AddTo(p.spec.ActiveFrom)

// If the selected phase is before the current phase, it's forbidden
if phaseStartTime.Before(currentPhaseStartTime) {
return nil, 0, &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot change contents of phase %s which starts before current phase", phaseKey)}
} else if phase.PhaseKey == currentPhase.PhaseKey {
return phase, isCurrentPhase, nil
} else if phaseStartTime.After(currentPhaseStartTime) {
return phase, isFuturePhase, nil
} else {
return nil, 0, fmt.Errorf("didn't enter any logical branch")
}
}
}

type relativeCadenceHelper struct {
contentType string
phaseStartTime time.Time
phaseKey string
rel timeRelation
actx subscription.ApplyContext
}

func (r relativeCadenceHelper) ValidateRelativeCadence(c *subscription.CadenceOverrideRelativeToPhaseStart) error {
if r.rel == isCurrentPhase {
// 2. If it's added to the current phase, the specified start time cannot point to the past
if c.ActiveFromOverride != nil {
iST, _ := c.ActiveFromOverride.AddTo(r.phaseStartTime)
if iST.Before(r.actx.CurrentTime) {
return &subscription.PatchForbiddenError{Msg: fmt.Sprintf("cannot add %s to phase %s which would become active in the past at %s", r.contentType, r.phaseKey, iST)}
}
} else {
// 3. If it's added to the current phase, and start time is not specified, it will be set for the current time, as you cannot change the past
diff := datex.Between(r.phaseStartTime, r.actx.CurrentTime)
c.ActiveFromOverride = &diff
}
}

return nil
}
39 changes: 39 additions & 0 deletions openmeter/subscription/phase.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,48 @@ type SubscriptionPhase struct {

// Description
Description *string `json:"description,omitempty"`

// OrderedDiscountIDs is the list of discount IDs that are applied in order.
OrderedDiscountIDs []string `json:"orderedDiscountIds"`
}

type CadenceOverrideRelativeToPhaseStart struct {
ActiveFromOverride *datex.Period `json:"activeFromOverrideRelativeToPhaseStart"`
ActiveToOverride *datex.Period `json:"activeToOverrideRelativeToPhaseStart,omitempty"`
}

func (c CadenceOverrideRelativeToPhaseStart) GetCadence(base models.CadencedModel) models.CadencedModel {
start := base.ActiveFrom
if c.ActiveFromOverride != nil {
start, _ = c.ActiveFromOverride.AddTo(base.ActiveFrom)
}

if base.ActiveTo != nil {
if base.ActiveTo.Before(start) {
// If the intended start time is after the intended end time of the phase, the item will have 0 lifetime at the end of the phase
// This scenario is possible when Subscriptions are canceled (before the phase ends)
return models.CadencedModel{
ActiveFrom: *base.ActiveTo,
ActiveTo: base.ActiveTo,
}
}
}

end := base.ActiveTo

if c.ActiveToOverride != nil {
endTime, _ := c.ActiveToOverride.AddTo(base.ActiveFrom)

if base.ActiveTo != nil && base.ActiveTo.Before(endTime) {
// Phase Cadence overrides item cadence in all cases
endTime = *base.ActiveTo
}

end = &endTime
}

return models.CadencedModel{
ActiveFrom: start,
ActiveTo: end,
}
}
34 changes: 1 addition & 33 deletions openmeter/subscription/subscriptionspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,39 +416,7 @@ type SubscriptionItemSpec struct {
}

func (s SubscriptionItemSpec) GetCadence(phaseCadence models.CadencedModel) (models.CadencedModel, error) {
start := phaseCadence.ActiveFrom
if s.CadenceOverrideRelativeToPhaseStart.ActiveFromOverride != nil {
start, _ = s.CadenceOverrideRelativeToPhaseStart.ActiveFromOverride.AddTo(phaseCadence.ActiveFrom)
}

if phaseCadence.ActiveTo != nil {
if phaseCadence.ActiveTo.Before(start) {
// If the intended start time is after the intended end time of the phase, the item will have 0 lifetime at the end of the phase
// This scenario is possible when Subscriptions are canceled (before the phase ends)
return models.CadencedModel{
ActiveFrom: *phaseCadence.ActiveTo,
ActiveTo: phaseCadence.ActiveTo,
}, nil
}
}

end := phaseCadence.ActiveTo

if s.CadenceOverrideRelativeToPhaseStart.ActiveToOverride != nil {
endTime, _ := s.CadenceOverrideRelativeToPhaseStart.ActiveToOverride.AddTo(phaseCadence.ActiveFrom)

if phaseCadence.ActiveTo != nil && phaseCadence.ActiveTo.Before(endTime) {
// Phase Cadence overrides item cadence in all cases
endTime = *phaseCadence.ActiveTo
}

end = &endTime
}

return models.CadencedModel{
ActiveFrom: start,
ActiveTo: end,
}, nil
return s.CreateSubscriptionItemCustomerInput.CadenceOverrideRelativeToPhaseStart.GetCadence(phaseCadence), nil
}

func (s SubscriptionItemSpec) ToCreateSubscriptionItemEntityInput(
Expand Down

0 comments on commit bb3665b

Please sign in to comment.