Skip to content

Commit 5c771be

Browse files
authored
fix(controlplane): enforce cache-invalidating project repo and wire bootstrap licenser (#2704)
wire the api layer's ProjectRepo through a single cached repository so every write path invalidates the projects:<id> cache key. previously several handlers constructed their own uncached repo, so config updates via the api were served stale for up to the cache ttl. also pass the licenser into CreateOrganisationService from the bootstrap command; it was omitted and caused a nil-pointer panic on convoy bootstrap, and guard the service against a nil licenser with a clear error.
1 parent 1f1a2be commit 5c771be

17 files changed

Lines changed: 148 additions & 33 deletions

api/api_options_repositories.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@ package api
22

33
import (
44
"github.com/frain-dev/convoy/api/types"
5+
"github.com/frain-dev/convoy/datastore/cached"
56
"github.com/frain-dev/convoy/internal/organisation_members"
67
"github.com/frain-dev/convoy/internal/organisations"
78
"github.com/frain-dev/convoy/internal/projects"
89
)
910

1011
// ensureAPIRepositories wires organisation and project repositories when the caller
1112
// omitted them but provided DB and Logger (e.g. cmd/server or dataplane startup).
13+
// The project repository is wrapped in the read-through cache whenever a cache is
14+
// available so every write path invalidates "projects:<id>" instead of serving
15+
// stale config until the TTL expires.
1216
func ensureAPIRepositories(a *types.APIOptions) {
1317
if a == nil || a.DB == nil || a.Logger == nil {
1418
return
@@ -20,6 +24,11 @@ func ensureAPIRepositories(a *types.APIOptions) {
2024
a.OrgMemberRepo = organisation_members.New(a.Logger, a.DB)
2125
}
2226
if a.ProjectRepo == nil {
23-
a.ProjectRepo = projects.New(a.Logger, a.DB)
27+
projectRepo := projects.New(a.Logger, a.DB)
28+
if a.Cache != nil {
29+
a.ProjectRepo = cached.NewCachedProjectRepository(projectRepo, a.Cache, cached.DefaultProjectTTL, a.Logger)
30+
} else {
31+
a.ProjectRepo = projectRepo
32+
}
2433
}
2534
}

api/api_options_repositories_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"go.uber.org/mock/gomock"
99

1010
"github.com/frain-dev/convoy/api/types"
11+
"github.com/frain-dev/convoy/datastore/cached"
1112
"github.com/frain-dev/convoy/mocks"
1213
"github.com/frain-dev/convoy/pkg/logger"
1314
)
@@ -54,6 +55,49 @@ func TestEnsureAPIRepositories_preservesExplicitRepos(t *testing.T) {
5455
require.NotNil(t, a.ProjectRepo)
5556
}
5657

58+
// Regression: project config updates via the API must invalidate the
59+
// "projects:<id>" cache key. That only holds when the wired ProjectRepo is the
60+
// cache-invalidating repository, since handlers write through it.
61+
func TestEnsureAPIRepositories_wrapsProjectRepoInCacheWhenAvailable(t *testing.T) {
62+
ctrl := gomock.NewController(t)
63+
db := mocks.NewMockDatabase(ctrl)
64+
db.EXPECT().GetConn().Return(nil).AnyTimes()
65+
db.EXPECT().GetHook().Return(nil).AnyTimes()
66+
67+
lo := logger.New("test", slog.LevelError)
68+
69+
a := &types.APIOptions{
70+
DB: db,
71+
Logger: lo,
72+
Cache: mocks.NewMockCache(ctrl),
73+
}
74+
75+
ensureAPIRepositories(a)
76+
77+
require.IsType(t, &cached.CachedProjectRepository{}, a.ProjectRepo,
78+
"ProjectRepo must be the cache-invalidating repository when a cache is available")
79+
}
80+
81+
func TestEnsureAPIRepositories_plainProjectRepoWithoutCache(t *testing.T) {
82+
ctrl := gomock.NewController(t)
83+
db := mocks.NewMockDatabase(ctrl)
84+
db.EXPECT().GetConn().Return(nil).AnyTimes()
85+
db.EXPECT().GetHook().Return(nil).AnyTimes()
86+
87+
lo := logger.New("test", slog.LevelError)
88+
89+
a := &types.APIOptions{
90+
DB: db,
91+
Logger: lo,
92+
}
93+
94+
ensureAPIRepositories(a)
95+
96+
require.NotNil(t, a.ProjectRepo)
97+
_, isCached := a.ProjectRepo.(*cached.CachedProjectRepository)
98+
require.False(t, isCached, "no cache available, so the plain repository is wired")
99+
}
100+
57101
func TestEnsureAPIRepositories_noOpWithoutDB(t *testing.T) {
58102
lo := logger.New("test", slog.LevelError)
59103
a := &types.APIOptions{Logger: lo}

api/handlers/endpoint.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"github.com/frain-dev/convoy/internal/event_deliveries"
1919
"github.com/frain-dev/convoy/internal/pkg/cbenablement"
2020
"github.com/frain-dev/convoy/internal/pkg/middleware"
21-
"github.com/frain-dev/convoy/internal/projects"
2221
convoynet "github.com/frain-dev/convoy/net"
2322
"github.com/frain-dev/convoy/pkg/circuit_breaker"
2423
"github.com/frain-dev/convoy/pkg/constants"
@@ -97,7 +96,7 @@ func (h *Handler) CreateEndpoint(w http.ResponseWriter, r *http.Request) {
9796

9897
ce := services.CreateEndpointService{
9998
EndpointRepo: endpointsvc.New(h.A.Logger, h.A.DB),
100-
ProjectRepo: projects.New(h.A.Logger, h.A.DB),
99+
ProjectRepo: h.projectRepo(),
101100
Licenser: h.A.Licenser,
102101
E: e,
103102
ProjectID: project.UID,
@@ -481,7 +480,7 @@ func (h *Handler) UpdateEndpoint(w http.ResponseWriter, r *http.Request) {
481480
ce := services.UpdateEndpointService{
482481
Cache: h.A.Cache,
483482
EndpointRepo: endpointsvc.New(h.A.Logger, h.A.DB),
484-
ProjectRepo: projects.New(h.A.Logger, h.A.DB),
483+
ProjectRepo: h.projectRepo(),
485484
Licenser: h.A.Licenser,
486485
FeatureFlag: h.A.FFlag,
487486
FeatureFlagFetcher: h.A.FeatureFlagFetcher,
@@ -612,7 +611,7 @@ func (h *Handler) ExpireSecret(w http.ResponseWriter, r *http.Request) {
612611
Queuer: h.A.Queue,
613612
Cache: h.A.Cache,
614613
EndpointRepo: endpointsvc.New(h.A.Logger, h.A.DB),
615-
ProjectRepo: projects.New(h.A.Logger, h.A.DB),
614+
ProjectRepo: h.projectRepo(),
616615
S: e,
617616
Endpoint: endpoint,
618617
Project: project,

api/handlers/handlers.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"github.com/frain-dev/convoy/auth"
1616
"github.com/frain-dev/convoy/config"
1717
"github.com/frain-dev/convoy/datastore"
18-
"github.com/frain-dev/convoy/datastore/cached"
1918
"github.com/frain-dev/convoy/internal/events"
2019
"github.com/frain-dev/convoy/internal/organisation_members"
2120
"github.com/frain-dev/convoy/internal/organisations"
@@ -41,7 +40,10 @@ func (h *Handler) orgRepo() datastore.OrganisationRepository {
4140
}
4241

4342
// projectRepo returns the injected project repository, falling back to a freshly
44-
// constructed one when none was wired (e.g. in tests).
43+
// constructed one when none was wired (e.g. in tests). In production wiring
44+
// (ensureAPIRepositories) this is the cached repository, so all project reads
45+
// share the "projects:<id>" cache and all writes invalidate it. Handlers must
46+
// use this accessor instead of constructing their own project repository.
4547
func (h *Handler) projectRepo() datastore.ProjectRepository {
4648
if h.A.ProjectRepo != nil {
4749
return h.A.ProjectRepo
@@ -95,7 +97,7 @@ func (h *Handler) retrieveProject(r *http.Request) (*datastore.Project, error) {
9597
var project *datastore.Project
9698
var err error
9799

98-
projectRepo := cached.NewCachedProjectRepository(projects.New(h.A.Logger, h.A.DB), h.A.Cache, 5*time.Minute, h.A.Logger)
100+
projectRepo := h.projectRepo()
99101

100102
switch {
101103
case h.IsReqWithJWT(authUser), h.IsReqWithPersonalAccessToken(authUser):
@@ -205,7 +207,7 @@ func (h *Handler) retrieveOrganisation(r *http.Request) (*datastore.Organisation
205207
orgRepo := organisations.New(h.A.Logger, h.A.DB)
206208
org, err = orgRepo.FetchOrganisationByID(r.Context(), project.OrganisationID)
207209
} else if projectID := chi.URLParam(r, "projectID"); projectID != "" {
208-
projectRepo := projects.New(h.A.Logger, h.A.DB)
210+
projectRepo := h.projectRepo()
209211
var project *datastore.Project
210212
project, err = projectRepo.FetchProjectByID(r.Context(), projectID)
211213
if err == nil {

api/handlers/organisation.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"github.com/frain-dev/convoy/internal/pkg/cbenablement"
2626
fflag "github.com/frain-dev/convoy/internal/pkg/fflag"
2727
m "github.com/frain-dev/convoy/internal/pkg/middleware"
28-
"github.com/frain-dev/convoy/internal/projects"
2928
"github.com/frain-dev/convoy/services"
3029
"github.com/frain-dev/convoy/util"
3130
"github.com/frain-dev/convoy/worker/task"
@@ -675,7 +674,7 @@ func (h *Handler) GetOrganisationCircuitBreakerConfig(w http.ResponseWriter, r *
675674
return
676675
}
677676

678-
projectRepo := projects.New(h.A.Logger, h.A.DB)
677+
projectRepo := h.projectRepo()
679678
projects, err := projectRepo.LoadProjects(r.Context(), &datastore.ProjectFilter{OrgID: orgID})
680679
if err != nil {
681680
_ = render.Render(w, r, util.NewServiceErrResponse(err))
@@ -747,7 +746,7 @@ func (h *Handler) UpdateOrganisationCircuitBreakerConfig(w http.ResponseWriter,
747746
return
748747
}
749748

750-
projectRepo := projects.New(h.A.Logger, h.A.DB)
749+
projectRepo := h.projectRepo()
751750
projects, err := projectRepo.LoadProjects(r.Context(), &datastore.ProjectFilter{OrgID: orgID})
752751
if err != nil {
753752
_ = render.Render(w, r, util.NewServiceErrResponse(err))
@@ -818,7 +817,7 @@ func (h *Handler) GetProjectCircuitBreakerConfig(w http.ResponseWriter, r *http.
818817
return
819818
}
820819

821-
projectRepo := projects.New(h.A.Logger, h.A.DB)
820+
projectRepo := h.projectRepo()
822821
project, err := projectRepo.FetchProjectByID(r.Context(), projectID)
823822
if err != nil {
824823
if errors.Is(err, datastore.ErrProjectNotFound) {
@@ -877,7 +876,7 @@ func (h *Handler) UpdateProjectCircuitBreakerConfig(w http.ResponseWriter, r *ht
877876
return
878877
}
879878

880-
projectRepo := projects.New(h.A.Logger, h.A.DB)
879+
projectRepo := h.projectRepo()
881880
project, err := projectRepo.FetchProjectByID(r.Context(), projectID)
882881
if err != nil {
883882
if errors.Is(err, datastore.ErrProjectNotFound) {

api/handlers/project.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/frain-dev/convoy/internal/event_deliveries"
1313
"github.com/frain-dev/convoy/internal/event_types"
1414
"github.com/frain-dev/convoy/internal/events"
15-
"github.com/frain-dev/convoy/internal/projects"
1615
"github.com/frain-dev/convoy/services"
1716
"github.com/frain-dev/convoy/util"
1817
)
@@ -21,7 +20,10 @@ const errBillingRequired = "complete billing setup to create projects: add a sub
2120

2221
func createProjectService(h *Handler) *services.ProjectService {
2322
apiKeyRepo := api_keys.New(h.A.Logger, h.A.DB)
24-
projectRepo := projects.New(h.A.Logger, h.A.DB)
23+
// Must be the cache-invalidating repository: ProjectService.UpdateProject
24+
// persists config changes (meta events URL, signature versions, etc.) that
25+
// the API and dataplane read through the "projects:<id>" cache.
26+
projectRepo := h.projectRepo()
2527
eventRepo := events.New(h.A.Logger, h.A.DB)
2628
eventDeliveryRepo := event_deliveries.New(h.A.Logger, h.A.DB)
2729
eventTypesRepo := event_types.New(h.A.Logger, h.A.DB)
@@ -57,7 +59,7 @@ func (h *Handler) GetProjectStatistics(w http.ResponseWriter, r *http.Request) {
5759
return
5860
}
5961

60-
err = projects.New(h.A.Logger, h.A.DB).FillProjectsStatistics(r.Context(), project)
62+
err = h.projectRepo().FillProjectsStatistics(r.Context(), project)
6163
if err != nil {
6264
h.A.Logger.ErrorContext(r.Context(), "failed to count project statistics", "error", err)
6365
_ = render.Render(w, r, util.NewErrorResponse("failed to count project statistics", http.StatusBadRequest))
@@ -79,7 +81,7 @@ func (h *Handler) DeleteProject(w http.ResponseWriter, r *http.Request) {
7981
return
8082
}
8183

82-
err = projects.New(h.A.Logger, h.A.DB).DeleteProject(r.Context(), project.UID)
84+
err = h.projectRepo().DeleteProject(r.Context(), project.UID)
8385
if err != nil {
8486
h.A.Logger.ErrorContext(r.Context(), "failed to delete project", "error", err)
8587
_ = render.Render(w, r, util.NewErrorResponse("failed to delete project", http.StatusBadRequest))
@@ -137,7 +139,7 @@ func (h *Handler) CreateProject(w http.ResponseWriter, r *http.Request) {
137139
}
138140
limitDeps := services.OrgProjectLimitDeps{
139141
BillingClient: h.A.BillingClient,
140-
ProjectRepo: projects.New(h.A.Logger, h.A.DB),
142+
ProjectRepo: h.projectRepo(),
141143
Cfg: h.A.Cfg,
142144
Logger: h.A.Logger,
143145
}
@@ -215,7 +217,7 @@ func (h *Handler) GetProjects(w http.ResponseWriter, r *http.Request) {
215217
}
216218

217219
filter := &datastore.ProjectFilter{OrgID: org.UID}
218-
projectsList, err := projects.New(h.A.Logger, h.A.DB).LoadProjects(r.Context(), filter)
220+
projectsList, err := h.projectRepo().LoadProjects(r.Context(), filter)
219221
if err != nil {
220222
h.A.Logger.ErrorContext(r.Context(), "failed to load projects", "error", err)
221223
_ = render.Render(w, r, util.NewErrorResponse("an error occurred while fetching projects", http.StatusBadRequest))

api/handlers/project_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ import (
88
"testing"
99

1010
"github.com/stretchr/testify/require"
11+
"go.uber.org/mock/gomock"
1112

1213
"github.com/frain-dev/convoy/api/types"
1314
"github.com/frain-dev/convoy/config"
15+
"github.com/frain-dev/convoy/mocks"
1416
log "github.com/frain-dev/convoy/pkg/logger"
1517
)
1618

@@ -35,3 +37,30 @@ func TestHandler_CreateProject_InvalidBody_Returns400(t *testing.T) {
3537
require.NoError(t, err)
3638
require.Contains(t, response["message"], "Invalid request format")
3739
}
40+
41+
// Regression: ProjectService.UpdateProject persists config the API and
42+
// dataplane read through the "projects:<id>" cache, so the service must be
43+
// built on the shared (cache-invalidating) repository from h.projectRepo(),
44+
// never on a freshly constructed uncached one that skips invalidation.
45+
func TestCreateProjectService_UsesSharedProjectRepo(t *testing.T) {
46+
ctrl := gomock.NewController(t)
47+
48+
db := mocks.NewMockDatabase(ctrl)
49+
db.EXPECT().GetConn().Return(nil).AnyTimes()
50+
db.EXPECT().GetHook().Return(nil).AnyTimes()
51+
52+
projectRepo := mocks.NewMockProjectRepository(ctrl)
53+
54+
handler := &Handler{
55+
A: &types.APIOptions{
56+
Logger: log.New("convoy", log.LevelInfo),
57+
DB: db,
58+
ProjectRepo: projectRepo,
59+
},
60+
}
61+
62+
svc := createProjectService(handler)
63+
64+
require.Same(t, projectRepo, svc.ProjectRepo,
65+
"createProjectService must reuse the wired project repository")
66+
}

api/handlers/security.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/frain-dev/convoy/datastore"
1313
"github.com/frain-dev/convoy/internal/api_keys"
1414
m "github.com/frain-dev/convoy/internal/pkg/middleware"
15-
"github.com/frain-dev/convoy/internal/projects"
1615
"github.com/frain-dev/convoy/internal/users"
1716
"github.com/frain-dev/convoy/services"
1817
"github.com/frain-dev/convoy/util"
@@ -33,7 +32,7 @@ func (h *Handler) CreatePersonalAPIKey(w http.ResponseWriter, r *http.Request) {
3332
}
3433

3534
cpk := &services.CreatePersonalAPIKeyService{
36-
ProjectRepo: projects.New(h.A.Logger, h.A.DB),
35+
ProjectRepo: h.projectRepo(),
3736
UserRepo: users.New(h.A.Logger, h.A.DB),
3837
APIKeyRepo: api_keys.New(h.A.Logger, h.A.DB),
3938
User: user,
@@ -75,7 +74,7 @@ func (h *Handler) RevokePersonalAPIKey(w http.ResponseWriter, r *http.Request) {
7574
}
7675

7776
rvk := &services.RevokePersonalAPIKeyService{
78-
ProjectRepo: projects.New(h.A.Logger, h.A.DB),
77+
ProjectRepo: h.projectRepo(),
7978
UserRepo: users.New(h.A.Logger, h.A.DB),
8079
APIKeyRepo: api_keys.New(h.A.Logger, h.A.DB),
8180
UID: chi.URLParam(r, "keyID"),
@@ -111,7 +110,7 @@ func (h *Handler) RegenerateProjectAPIKey(w http.ResponseWriter, r *http.Request
111110
}
112111

113112
rgp := &services.RegenerateProjectAPIKeyService{
114-
ProjectRepo: projects.New(h.A.Logger, h.A.DB),
113+
ProjectRepo: h.projectRepo(),
115114
UserRepo: users.New(h.A.Logger, h.A.DB),
116115
APIKeyRepo: api_keys.New(h.A.Logger, h.A.DB),
117116
Project: project,

api/handlers/subscription.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/frain-dev/convoy/internal/endpoints"
1313
"github.com/frain-dev/convoy/internal/organisations"
1414
"github.com/frain-dev/convoy/internal/pkg/middleware"
15-
"github.com/frain-dev/convoy/internal/projects"
1615
"github.com/frain-dev/convoy/internal/sources"
1716
"github.com/frain-dev/convoy/internal/subscriptions"
1817
"github.com/frain-dev/convoy/pkg/transform"
@@ -380,7 +379,7 @@ func (h *Handler) UpdateSubscription(w http.ResponseWriter, r *http.Request) {
380379
us := services.UpdateSubscriptionService{
381380
SubRepo: subscriptions.New(h.A.Logger, h.A.DB),
382381
EndpointRepo: endpoints.New(h.A.Logger, h.A.DB),
383-
ProjectRepo: projects.New(h.A.Logger, h.A.DB),
382+
ProjectRepo: h.projectRepo(),
384383
SourceRepo: sources.New(h.A.Logger, h.A.DB),
385384
Licenser: h.A.Licenser,
386385
ProjectId: project.UID,

api/ingest.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"github.com/frain-dev/convoy/internal/pkg/crc"
2424
"github.com/frain-dev/convoy/internal/pkg/dedup"
2525
"github.com/frain-dev/convoy/internal/pkg/license"
26-
"github.com/frain-dev/convoy/internal/projects"
2726
"github.com/frain-dev/convoy/internal/sources"
2827
"github.com/frain-dev/convoy/pkg/httpheader"
2928
"github.com/frain-dev/convoy/pkg/msgpack"
@@ -51,8 +50,8 @@ func (a *ApplicationHandler) IngestEvent(w http.ResponseWriter, r *http.Request)
5150
}
5251

5352
// 2. Retrieve source using mask ID.
54-
projectRepo := projects.New(a.A.Logger, a.A.DB)
55-
project, err := projectRepo.FetchProjectByID(r.Context(), source.ProjectID)
53+
// ProjectRepo is the cached repository wired by ensureAPIRepositories.
54+
project, err := a.A.ProjectRepo.FetchProjectByID(r.Context(), source.ProjectID)
5655
if err != nil {
5756
_ = render.Render(w, r, util.NewServiceErrResponse(err))
5857
return

0 commit comments

Comments
 (0)