Skip to content

Commit 58fb18f

Browse files
authored
fix: recurring period calculation fix (#3224)
1 parent 5bb112c commit 58fb18f

File tree

14 files changed

+550
-138
lines changed

14 files changed

+550
-138
lines changed

Makefile

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,12 @@ test: ## Run tests
140140
PGPASSWORD=postgres psql -h 127.0.0.1 -U postgres postgres -c "SELECT version();" || (echo "!!! Postgres is not running. Please start it with 'docker compose up -d postgres' !!!" && false)
141141
go test ${GO_TEST_FLAGS} ./...
142142

143+
.PHONY: test-nocache
144+
test-nocache: ## Run tests without cache
145+
$(call print-target)
146+
PGPASSWORD=postgres psql -h 127.0.0.1 -U postgres postgres -c "SELECT version();" || (echo "!!! Postgres is not running. Please start it with 'docker compose up -d postgres' !!!" && false)
147+
go test ${GO_TEST_FLAGS} -count=1 ./...
148+
143149
.PHONY: test-all
144150
test-all: ## Run tests with svix dependencies, bypassing the test cache
145151
$(call print-target)

openmeter/credit/engine/engine.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,3 @@ type engine struct {
6161

6262
// Ensure engine implements Engine
6363
var _ Engine = (*engine)(nil)
64-
65-
func later(t1 time.Time, t2 time.Time) time.Time {
66-
if t1.After(t2) {
67-
return t1
68-
}
69-
return t2
70-
}

openmeter/credit/engine/grant.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,20 @@ func (e *engine) getGrantRecurrenceTimes(grants []grant.Grant, period timeutil.C
7575
}
7676

7777
for _, grant := range grantsWithRecurrence {
78-
i, err := grant.Recurrence.NextAfter(later(grant.EffectiveAt, period.From))
78+
it, err := grant.Recurrence.IterateFromNextAfter(
79+
timeutil.Later(grant.EffectiveAt, period.From),
80+
timeutil.Inclusive,
81+
)
7982
if err != nil {
8083
return nil, err
8184
}
8285
// writing all reccurence times until grant is active or period ends
83-
for i.Before(period.To) && grant.ActiveAt(i) {
86+
for it.At.Before(period.To) && grant.ActiveAt(it.At) {
8487
times = append(times, struct {
8588
time time.Time
8689
grantID string
87-
}{time: i, grantID: grant.ID})
88-
i, err = grant.Recurrence.Next(i)
90+
}{time: it.At, grantID: grant.ID})
91+
it, err = it.Next()
8992
if err != nil {
9093
return nil, err
9194
}

openmeter/entitlement/entitlement.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ func (c CreateEntitlementInputs) Validate() error {
206206

207207
// Let's validate the Usage Period
208208
if c.UsagePeriod != nil {
209-
if per, err := c.UsagePeriod.GetValue().Interval.ISODuration.Subtract(datetime.NewISODuration(0, 0, 0, 0, 1, 0, 0)); err == nil && per.Sign() == -1 {
209+
if per, err := c.UsagePeriod.GetValue().Interval.ISODuration.Subtract(datetime.DurationHour); err == nil && per.Sign() == -1 {
210210
return fmt.Errorf("UsagePeriod must be at least 1 hour")
211211
}
212212
}

openmeter/entitlement/metered/entitlement_grant.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/openmeterio/openmeter/pkg/clock"
1313
"github.com/openmeterio/openmeter/pkg/convert"
1414
"github.com/openmeterio/openmeter/pkg/models"
15+
"github.com/openmeterio/openmeter/pkg/timeutil"
1516
)
1617

1718
// CreateGrant creates a grant for a given entitlement
@@ -109,7 +110,7 @@ type EntitlementGrant struct {
109110
func GrantFromCreditGrant(grant grant.Grant) (*EntitlementGrant, error) {
110111
g := &EntitlementGrant{}
111112
if grant.Recurrence != nil {
112-
next, err := grant.Recurrence.NextAfter(clock.Now())
113+
next, err := grant.Recurrence.NextAfter(clock.Now(), timeutil.Inclusive)
113114
if err != nil {
114115
return nil, err
115116
}

openmeter/subscription/subscriptionspec.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func (s *SubscriptionSpec) GetAlignedBillingPeriodAt(at time.Time) (timeutil.Clo
253253
// TODO(galexi, OM-1418): implement reanchoring
254254

255255
// We will use the subscription billing anchor as the cadence anchor
256-
billingRecurrence, err := timeutil.RecurrenceFromISODuration(lo.ToPtr(s.BillingCadence), s.BillingAnchor)
256+
billingRecurrence, err := timeutil.NewRecurrenceFromISODuration(s.BillingCadence, s.BillingAnchor)
257257
if err != nil {
258258
return def, fmt.Errorf("failed to get billing recurrence for phase %s: %w", phase.PhaseKey, err)
259259
}
@@ -815,7 +815,7 @@ func (s SubscriptionItemSpec) GetFullServicePeriodAt(
815815
}, nil
816816
}
817817

818-
rec, err := timeutil.RecurrenceFromISODuration(billingCadence, alignedBillingAnchor)
818+
rec, err := timeutil.NewRecurrenceFromISODuration(*billingCadence, alignedBillingAnchor)
819819
if err != nil {
820820
return timeutil.ClosedPeriod{}, fmt.Errorf("failed to get recurrence from ISO duration: %w", err)
821821
}
@@ -922,7 +922,7 @@ func (s SubscriptionItemSpec) ToScheduleSubscriptionEntitlementInput(
922922
scheduleInput.IssueAfterReset = tpl.IssueAfterReset
923923
scheduleInput.IssueAfterResetPriority = tpl.IssueAfterResetPriority
924924
scheduleInput.PreserveOverageAtReset = tpl.PreserveOverageAtReset
925-
rec, err := timeutil.RecurrenceFromISODuration(&tpl.UsagePeriod, truncatedAnchorTime)
925+
rec, err := timeutil.NewRecurrenceFromISODuration(tpl.UsagePeriod, truncatedAnchorTime)
926926
if err != nil {
927927
return def, true, fmt.Errorf("failed to get recurrence from ISO duration: %w", err)
928928
}

openmeter/subscription/workflow/service/subscription.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,26 +52,16 @@ func (s *service) CreateFromPlan(ctx context.Context, inp subscriptionworkflow.C
5252
// Let's normalize the billing anchor to the closest iteration based on the cadence
5353
billingAnchor := lo.FromPtrOr(inp.BillingAnchor, activeFrom).UTC()
5454

55-
billingRecurrence, err := timeutil.RecurrenceFromISODuration(lo.ToPtr(plan.ToCreateSubscriptionPlanInput().BillingCadence), billingAnchor)
55+
billingRecurrence, err := timeutil.NewRecurrenceFromISODuration(plan.ToCreateSubscriptionPlanInput().BillingCadence, billingAnchor)
5656
if err != nil {
5757
return def, fmt.Errorf("failed to get billing recurrence: %w", err)
5858
}
5959

60-
billingAnchor, err = billingRecurrence.PrevBefore(activeFrom)
60+
billingAnchor, err = billingRecurrence.PrevBefore(activeFrom, timeutil.Inclusive)
6161
if err != nil {
6262
return def, fmt.Errorf("failed to get billing anchor: %w", err)
6363
}
6464

65-
// When anchor = beforeTime (or falls on iteration boundary), PrevBefore will return an iteration early.
66-
oneBefore, err := billingRecurrence.Prev(activeFrom)
67-
if err != nil {
68-
return def, fmt.Errorf("failed to get one iteration before billing anchor: %w", err)
69-
}
70-
71-
if oneBefore.Equal(billingAnchor) {
72-
billingAnchor = activeFrom
73-
}
74-
7565
// Let's create the new Spec
7666
spec, err := subscription.NewSpecFromPlan(plan, subscription.CreateSubscriptionCustomerInput{
7767
CustomerId: cust.ID,

openmeter/subscription/workflow/service/subscription_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,12 @@ func TestCreateFromPlan(t *testing.T) {
104104

105105
billingAnchor := activeFrom.Add(time.Hour)
106106

107-
cadRec, err := timeutil.RecurrenceFromISODuration(lo.ToPtr(cad), billingAnchor)
107+
cadRec, err := timeutil.NewRecurrenceFromISODuration(cad, billingAnchor)
108108
require.Nil(t, err)
109109

110110
// Let's set an anchor one hour after the activeFrom, which will then be set to one iteration before
111111

112-
expectedBillingAnchor, err := cadRec.Prev(billingAnchor)
112+
expectedBillingAnchor, err := cadRec.Iterator().Prev()
113113
require.Nil(t, err)
114114

115115
subView, err := deps.WorkflowService.CreateFromPlan(ctx, subscriptionworkflow.CreateSubscriptionWorkflowInput{

pkg/datetime/duration.go

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,42 +165,85 @@ func (d ISODuration) DivisibleBy(smaller ISODuration) (bool, error) {
165165
return true, nil
166166
}
167167

168+
func (p ISODuration) Mul(n int) (ISODuration, error) {
169+
nDecimal, err := decimal.NewFromInt64(int64(n), 0, 0)
170+
if err != nil {
171+
return ISODuration{}, err
172+
}
173+
174+
per, err := p.Period.Mul(nDecimal)
175+
if err != nil {
176+
return ISODuration{}, err
177+
}
178+
return ISODuration{per}, nil
179+
}
180+
168181
// convertPeriodToSeconds converts a period to total seconds using decimal precision
169182
func convertPeriodToSeconds(p period.Period, daysInMonth int, hoursInDays int) (decimal.Decimal, error) {
170-
zero := decimal.MustNew(0, 0)
183+
zero := decimal.Zero
184+
185+
yearsMul, err := decimal.New(int64(daysInMonth*12*hoursInDays*3600), 0)
186+
if err != nil {
187+
return zero, err
188+
}
171189

172190
// Convert years to seconds: years * (daysInMonth * 12) * hoursInDays * 3600
173-
years, err := p.YearsDecimal().Mul(decimal.MustNew(int64(daysInMonth*12*hoursInDays*3600), 0))
191+
years, err := p.YearsDecimal().Mul(yearsMul)
192+
if err != nil {
193+
return zero, err
194+
}
195+
196+
monthsMul, err := decimal.New(int64(daysInMonth*hoursInDays*3600), 0)
174197
if err != nil {
175198
return zero, err
176199
}
177200

178201
// Convert months to seconds: months * daysInMonth * hoursInDays * 3600
179-
months, err := p.MonthsDecimal().Mul(decimal.MustNew(int64(daysInMonth*hoursInDays*3600), 0))
202+
months, err := p.MonthsDecimal().Mul(monthsMul)
203+
if err != nil {
204+
return zero, err
205+
}
206+
207+
weeksMul, err := decimal.New(int64(7*hoursInDays*3600), 0)
180208
if err != nil {
181209
return zero, err
182210
}
183211

184212
// Convert weeks to seconds: weeks * 7 * hoursInDays * 3600
185-
weeks, err := p.WeeksDecimal().Mul(decimal.MustNew(int64(7*hoursInDays*3600), 0))
213+
weeks, err := p.WeeksDecimal().Mul(weeksMul)
186214
if err != nil {
187215
return zero, err
188216
}
189217

190218
// Convert days to seconds: days * hoursInDays * 3600
191-
days, err := p.DaysDecimal().Mul(decimal.MustNew(int64(hoursInDays*3600), 0))
219+
daysMul, err := decimal.New(int64(hoursInDays*3600), 0)
220+
if err != nil {
221+
return zero, err
222+
}
223+
224+
days, err := p.DaysDecimal().Mul(daysMul)
192225
if err != nil {
193226
return zero, err
194227
}
195228

196229
// Convert hours to seconds: hours * 3600
197-
hours, err := p.HoursDecimal().Mul(decimal.MustNew(3600, 0))
230+
hoursMul, err := decimal.New(int64(3600), 0)
231+
if err != nil {
232+
return zero, err
233+
}
234+
235+
hours, err := p.HoursDecimal().Mul(hoursMul)
198236
if err != nil {
199237
return zero, err
200238
}
201239

202240
// Convert minutes to seconds: minutes * 60
203-
minutes, err := p.MinutesDecimal().Mul(decimal.MustNew(60, 0))
241+
minutesMul, err := decimal.New(int64(60), 0)
242+
if err != nil {
243+
return zero, err
244+
}
245+
246+
minutes, err := p.MinutesDecimal().Mul(minutesMul)
204247
if err != nil {
205248
return zero, err
206249
}

pkg/datetime/interval.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package datetime
2+
3+
var (
4+
DurationSecond ISODuration = NewISODuration(0, 0, 0, 0, 0, 0, 1)
5+
DurationMinute ISODuration = NewISODuration(0, 0, 0, 0, 0, 1, 0)
6+
DurationHour ISODuration = NewISODuration(0, 0, 0, 0, 1, 0, 0)
7+
DurationDay ISODuration = NewISODuration(0, 0, 0, 1, 0, 0, 0)
8+
DurationWeek ISODuration = NewISODuration(0, 0, 1, 0, 0, 0, 0)
9+
DurationMonth ISODuration = NewISODuration(0, 1, 0, 0, 0, 0, 0)
10+
DurationYear ISODuration = NewISODuration(1, 0, 0, 0, 0, 0, 0)
11+
)

0 commit comments

Comments
 (0)