Skip to content

Commit 6ed4dab

Browse files
hekikehekike
authored
feat(billing): disallow deleting default profile (#2005)
1 parent a12649c commit 6ed4dab

File tree

4 files changed

+117
-46
lines changed

4 files changed

+117
-46
lines changed

‎openmeter/billing/errors.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@ var (
1616
// Given that invoicing depends on the billing and customer override service, we need to have these error types in place for
1717
// all.
1818

19-
ErrDefaultProfileAlreadyExists = NewValidationError("default_profile_exists", "default profile already exists")
20-
ErrDefaultProfileNotFound = NewValidationError("default_profile_not_found", "default profile not found")
21-
ErrProfileNotFound = NewValidationError("profile_not_found", "profile not found")
22-
ErrProfileAlreadyDeleted = NewValidationError("profile_already_deleted", "profile already deleted")
23-
ErrProfileReferencedByOverrides = NewValidationError("profile_referenced", "profile is referenced by customer overrides")
19+
ErrDefaultProfileAlreadyExists = NewValidationError("default_profile_exists", "default profile already exists")
20+
ErrDefaultProfileNotFound = NewValidationError("default_profile_not_found", "default profile not found")
21+
ErrProfileNotFound = NewValidationError("profile_not_found", "profile not found")
22+
ErrProfileAlreadyDeleted = NewValidationError("profile_already_deleted", "profile already deleted")
23+
ErrProfileReferencedByOverrides = NewValidationError("profile_referenced", "profile is referenced by customer overrides")
24+
ErrDefaultProfileCannotBeDeleted = NewValidationError("default_profile_cannot_be_deleted", "default profile cannot be deleted")
2425

2526
ErrCustomerOverrideNotFound = NewValidationError("customer_override_not_found", "customer override not found")
2627
ErrCustomerOverrideAlreadyDeleted = NewValidationError("customer_override_deleted", "customer override already deleted")

‎openmeter/billing/service/profile.go

+8
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,20 @@ func (s *Service) DeleteProfile(ctx context.Context, input billing.DeleteProfile
225225
}
226226
}
227227

228+
// Already deleted profiles cannot be deleted again
228229
if profile.DeletedAt != nil {
229230
return billing.ValidationError{
230231
Err: fmt.Errorf("%w [id=%s]", billing.ErrProfileAlreadyDeleted, profile.ID),
231232
}
232233
}
233234

235+
// Default profiles cannot be deleted
236+
if profile.Default {
237+
return billing.ValidationError{
238+
Err: fmt.Errorf("%w [id=%s]", billing.ErrDefaultProfileCannotBeDeleted, profile.ID),
239+
}
240+
}
241+
234242
referringCustomerIDs, err := s.adapter.GetCustomerOverrideReferencingProfile(ctx, billing.HasCustomerOverrideReferencingProfileAdapterInput(input))
235243
if err != nil {
236244
return err

‎test/billing/profile_test.go

+96-41
Original file line numberDiff line numberDiff line change
@@ -23,51 +23,73 @@ func TestProfile(t *testing.T) {
2323
suite.Run(t, new(ProfileTestSuite))
2424
}
2525

26-
func (s *ProfileTestSuite) TestProfileLifecycle() {
26+
// createProfileFixture creates a profile with the given default flag.
27+
// Note: only non default profiles can be deleted.
28+
func (s *ProfileTestSuite) createProfileFixture(isDefault bool) *billing.Profile {
29+
t := s.T()
2730
ctx := context.Background()
28-
ns := "test_create_billing_profile"
31+
ns := s.GetUniqueNamespace("test_billing_profile")
2932

30-
_ = s.InstallSandboxApp(s.T(), ns)
33+
// Create a profile input
34+
input := MinimalCreateProfileInputTemplate
35+
input.Namespace = ns
36+
input.Default = isDefault
37+
38+
// Create a sandbox app
39+
app := s.InstallSandboxApp(s.T(), ns)
40+
require.NotNil(t, app)
41+
42+
// Create a default profile
43+
profile, err := s.BillingService.CreateProfile(ctx, input)
44+
require.NoError(t, err)
45+
require.NotNil(t, profile)
46+
47+
profile.CreatedAt = profile.CreatedAt.Truncate(time.Microsecond)
48+
profile.UpdatedAt = profile.UpdatedAt.Truncate(time.Microsecond)
49+
profile.WorkflowConfig.CreatedAt = profile.WorkflowConfig.CreatedAt.Truncate(time.Microsecond)
50+
profile.WorkflowConfig.UpdatedAt = profile.WorkflowConfig.UpdatedAt.Truncate(time.Microsecond)
51+
52+
return profile
53+
}
54+
55+
func (s *ProfileTestSuite) TestProfileLifecycle() {
56+
ctx := context.Background()
3157

3258
s.T().Run("missing default profile", func(t *testing.T) {
59+
profile := s.createProfileFixture(false)
60+
3361
defaultProfile, err := s.BillingService.GetDefaultProfile(ctx, billing.GetDefaultProfileInput{
34-
Namespace: ns,
62+
Namespace: profile.Namespace,
3563
})
3664
require.NoError(t, err)
3765
require.Nil(t, defaultProfile)
3866
})
3967

40-
var profile *billing.Profile
41-
var err error
42-
43-
minimalCreateProfileInput := MinimalCreateProfileInputTemplate
44-
minimalCreateProfileInput.Namespace = ns
45-
4668
s.T().Run("create default profile", func(t *testing.T) {
47-
profile, err = s.BillingService.CreateProfile(ctx, minimalCreateProfileInput)
69+
profile := s.createProfileFixture(true)
4870

49-
require.NoError(t, err)
5071
require.NotNil(t, profile)
72+
require.True(t, profile.Default)
5173
})
5274

53-
profile.CreatedAt = profile.CreatedAt.Truncate(time.Microsecond)
54-
profile.UpdatedAt = profile.UpdatedAt.Truncate(time.Microsecond)
55-
profile.WorkflowConfig.CreatedAt = profile.WorkflowConfig.CreatedAt.Truncate(time.Microsecond)
56-
profile.WorkflowConfig.UpdatedAt = profile.WorkflowConfig.UpdatedAt.Truncate(time.Microsecond)
57-
5875
s.T().Run("fetching the default profile is possible", func(t *testing.T) {
76+
profile := s.createProfileFixture(true)
77+
5978
defaultProfile, err := s.BillingService.GetDefaultProfile(ctx, billing.GetDefaultProfileInput{
60-
Namespace: ns,
79+
Namespace: profile.Namespace,
6180
})
81+
6282
require.NoError(t, err)
6383
require.NotNil(t, defaultProfile)
6484
require.Equal(t, profile, defaultProfile)
6585
})
6686

6787
s.T().Run("fetching the profile by id", func(t *testing.T) {
88+
profile := s.createProfileFixture(false)
89+
6890
fetchedProfile, err := s.BillingService.GetProfile(ctx, billing.GetProfileInput{
6991
Profile: models.NamespacedID{
70-
Namespace: ns,
92+
Namespace: profile.Namespace,
7193
ID: profile.ID,
7294
},
7395
Expand: billing.ProfileExpand{
@@ -80,56 +102,89 @@ func (s *ProfileTestSuite) TestProfileLifecycle() {
80102
})
81103

82104
s.T().Run("creating a second default profile fails", func(t *testing.T) {
83-
_, err := s.BillingService.CreateProfile(ctx, minimalCreateProfileInput)
105+
profile := s.createProfileFixture(true)
106+
107+
// Try to create a second default profile in the same namespace
108+
input := MinimalCreateProfileInputTemplate
109+
input.Namespace = profile.Namespace
110+
111+
_, err := s.BillingService.CreateProfile(ctx, input)
84112
require.Error(t, err)
85113
require.ErrorIs(t, err, billing.ErrDefaultProfileAlreadyExists)
86114
})
87115

88116
s.T().Run("creating a second default profile succeeds with override", func(t *testing.T) {
89-
overrideInput := minimalCreateProfileInput
90-
overrideInput.DefaultOverride = true
117+
profile1 := s.createProfileFixture(true)
118+
119+
// Create a second default profile with override
120+
input := MinimalCreateProfileInputTemplate
121+
input.Namespace = profile1.Namespace
122+
input.DefaultOverride = true
91123

92-
profile, err := s.BillingService.CreateProfile(ctx, overrideInput)
124+
profile2, err := s.BillingService.CreateProfile(ctx, input)
93125
require.NoError(t, err)
126+
require.NotNil(t, profile2)
94127

95-
// Cleanup
96-
require.NoError(t, s.BillingService.DeleteProfile(ctx, billing.DeleteProfileInput{
97-
Namespace: ns,
98-
ID: profile.ID,
99-
}))
128+
require.NotEqual(t, profile1.ID, profile2.ID)
129+
require.True(t, profile2.Default)
100130
})
101131

102132
s.T().Run("deleted profile handling", func(t *testing.T) {
103-
require.NoError(t, s.BillingService.DeleteProfile(ctx, billing.DeleteProfileInput{
104-
Namespace: ns,
105-
ID: profile.ID,
106-
}))
133+
t.Run("deleting a profile succeeds", func(t *testing.T) {
134+
profile := s.createProfileFixture(false)
135+
136+
require.NoError(t, s.BillingService.DeleteProfile(ctx, billing.DeleteProfileInput{
137+
Namespace: profile.Namespace,
138+
ID: profile.ID,
139+
}))
140+
})
107141

108142
t.Run("deleting a profile twice yields an error", func(t *testing.T) {
143+
profile := s.createProfileFixture(false)
144+
145+
// Delete the profile
146+
require.NoError(t, s.BillingService.DeleteProfile(ctx, billing.DeleteProfileInput{
147+
Namespace: profile.Namespace,
148+
ID: profile.ID,
149+
}))
150+
151+
// Try to delete the profile again
109152
require.ErrorIs(t, s.BillingService.DeleteProfile(ctx, billing.DeleteProfileInput{
110-
Namespace: ns,
153+
Namespace: profile.Namespace,
111154
ID: profile.ID,
112155
}), billing.ErrProfileAlreadyDeleted)
113156
})
114157

158+
t.Run("deleting the default profile yields an error", func(t *testing.T) {
159+
profile := s.createProfileFixture(true)
160+
161+
// Try to delete the default profile
162+
require.ErrorIs(t, s.BillingService.DeleteProfile(ctx, billing.DeleteProfileInput{
163+
Namespace: profile.Namespace,
164+
ID: profile.ID,
165+
}), billing.ErrDefaultProfileCannotBeDeleted)
166+
})
167+
115168
t.Run("fetching a deleted profile by id returns the profile", func(t *testing.T) {
169+
profile := s.createProfileFixture(false)
170+
171+
// Delete the profile
172+
require.NoError(t, s.BillingService.DeleteProfile(ctx, billing.DeleteProfileInput{
173+
Namespace: profile.Namespace,
174+
ID: profile.ID,
175+
}))
176+
177+
// Fetch the profile
116178
fetchedProfile, err := s.BillingService.GetProfile(ctx, billing.GetProfileInput{
117179
Profile: models.NamespacedID{
118-
Namespace: ns,
180+
Namespace: profile.Namespace,
119181
ID: profile.ID,
120182
},
121183
})
122184

123185
require.NoError(t, err)
124186
require.Equal(t, profile.ID, fetchedProfile.ID)
125187
})
126-
127-
t.Run("same profile can be created after the previous one was deleted", func(t *testing.T) {
128-
profile, err = s.BillingService.CreateProfile(ctx, minimalCreateProfileInput)
129-
130-
require.NoError(t, err)
131-
require.NotNil(t, profile)
132-
})
133188
})
134189
}
135190

‎test/billing/suite.go

+7
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package billing
22

33
import (
44
"context"
5+
"fmt"
56
"log/slog"
67
"os"
78
"testing"
89

10+
"github.com/oklog/ulid/v2"
911
"github.com/stretchr/testify/require"
1012
"github.com/stretchr/testify/suite"
1113

@@ -53,6 +55,11 @@ type BaseSuite struct {
5355
SandboxApp *appsandbox.MockableFactory
5456
}
5557

58+
// GetUniqueNamespace returns a unique namespace with the given prefix
59+
func (s *BaseSuite) GetUniqueNamespace(prefix string) string {
60+
return fmt.Sprintf("%s_%s", prefix, ulid.Make().String())
61+
}
62+
5663
func (s *BaseSuite) SetupSuite() {
5764
t := s.T()
5865
t.Log("setup suite")

0 commit comments

Comments
 (0)