From c93ecfa4cf598109ee4847d8f36c652cea845473 Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Thu, 7 May 2026 19:35:00 +0200 Subject: [PATCH 01/14] implement generic in-memory cache and Gin middleware for response caching and invalidation --- internal/api/api.go | 30 +++- internal/api/errors/errors.go | 6 +- internal/api/routes.go | 29 ++-- internal/cache/cache.go | 87 +++++++++++ internal/cache/middleware.go | 77 ++++++++++ internal/cache/middleware_test.go | 238 ++++++++++++++++++++++++++++++ internal/db/db.go | 18 --- tests/cache_test.go | 161 ++++++++++++++++++++ 8 files changed, 609 insertions(+), 37 deletions(-) create mode 100644 internal/cache/cache.go create mode 100644 internal/cache/middleware.go create mode 100644 internal/cache/middleware_test.go create mode 100644 tests/cache_test.go diff --git a/internal/api/api.go b/internal/api/api.go index 1fd2afc..11e7259 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -2,23 +2,32 @@ package api import ( "fmt" + "time" "github.com/gin-gonic/gin" "go.uber.org/zap" "github.com/stackmon/otc-status-dashboard/internal/api/auth" "github.com/stackmon/otc-status-dashboard/internal/api/errors" + "github.com/stackmon/otc-status-dashboard/internal/cache" "github.com/stackmon/otc-status-dashboard/internal/conf" "github.com/stackmon/otc-status-dashboard/internal/db" ) +const ( + componentsCacheTTL = 60 * time.Second + eventsCacheTTL = 10 * time.Second +) + type API struct { - r *gin.Engine - db *db.DB - log *zap.Logger - oa2Prov *auth.Provider - secretKeyV1 string - authGroup string + r *gin.Engine + db *db.DB + log *zap.Logger + oa2Prov *auth.Provider + secretKeyV1 string + authGroup string + componentsCache *cache.Cache[cache.CachedResponse] + eventsCache *cache.Cache[cache.CachedResponse] } func New(cfg *conf.Config, log *zap.Logger, database *db.DB) (*API, error) { @@ -44,7 +53,14 @@ func New(cfg *conf.Config, log *zap.Logger, database *db.DB) (*API, error) { r.Use(CORSMiddleware()) r.NoRoute(errors.Return404) - a := &API{r: r, db: database, log: log, oa2Prov: oa2Prov, secretKeyV1: cfg.SecretKeyV1, authGroup: cfg.AuthGroup} + componentsCache := cache.NewHTTPCache(componentsCacheTTL) + eventsCache := cache.NewHTTPCache(eventsCacheTTL) + + a := &API{ + r: r, db: database, log: log, oa2Prov: oa2Prov, + secretKeyV1: cfg.SecretKeyV1, authGroup: cfg.AuthGroup, + componentsCache: componentsCache, eventsCache: eventsCache, + } a.InitRoutes() return a, nil } diff --git a/internal/api/errors/errors.go b/internal/api/errors/errors.go index ad1bc1b..657dcf9 100644 --- a/internal/api/errors/errors.go +++ b/internal/api/errors/errors.go @@ -2,10 +2,10 @@ package errors import ( "errors" + "fmt" "net/http" "github.com/gin-gonic/gin" - "go.uber.org/zap" ) func ReturnError(err error) error { @@ -28,8 +28,8 @@ func Return404(c *gin.Context) { } func RaiseInternalErr(c *gin.Context, err error) { - zap.L().Error("internal server error", zap.Error(err)) - c.AbortWithStatusJSON(http.StatusInternalServerError, ReturnError(ErrInternalError)) + intErr := fmt.Errorf("%w: %w", ErrInternalError, err) + c.AbortWithStatusJSON(http.StatusInternalServerError, ReturnError(intErr)) } func RaiseBadRequestErr(c *gin.Context, err error) { diff --git a/internal/api/routes.go b/internal/api/routes.go index 80191c2..9135fe1 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -5,6 +5,7 @@ import ( "github.com/stackmon/otc-status-dashboard/internal/api/rss" v1 "github.com/stackmon/otc-status-dashboard/internal/api/v1" v2 "github.com/stackmon/otc-status-dashboard/internal/api/v2" + "github.com/stackmon/otc-status-dashboard/internal/cache" newRSS "github.com/stackmon/otc-status-dashboard/internal/rss" ) @@ -26,74 +27,84 @@ func (a *API) InitRoutes() { v1API := a.r.Group(v1Group) { - v1API.GET("component_status", v1.GetComponentsStatusHandler(a.db, a.log)) + v1API.GET("component_status", cache.GinMiddleware(a.componentsCache), v1.GetComponentsStatusHandler(a.db, a.log)) v1API.POST("component_status", AuthenticationMW(a.oa2Prov, a.log, a.secretKeyV1, a.authGroup), + cache.Invalidator(a.componentsCache), v1.PostComponentStatusHandler(a.db, a.log), ) - v1API.GET("incidents", v1.GetIncidentsHandler(a.db, a.log)) + v1API.GET("incidents", cache.GinMiddleware(a.eventsCache), v1.GetIncidentsHandler(a.db, a.log)) } v2API := a.r.Group(v2Group) { - v2API.GET("components", v2.GetComponentsHandler(a.db, a.log)) + v2API.GET("components", cache.GinMiddleware(a.componentsCache), v2.GetComponentsHandler(a.db, a.log)) v2API.POST("components", AuthenticationMW( a.oa2Prov, a.log, a.secretKeyV1, a.authGroup, ), + cache.Invalidator(a.componentsCache), v2.PostComponentHandler(a.db, a.log)) - v2API.GET("components/:id", v2.GetComponentHandler(a.db, a.log)) + v2API.GET("components/:id", cache.GinMiddleware(a.componentsCache), v2.GetComponentHandler(a.db, a.log)) // Incidents section. Deprecated. // will be removed in a later version. - v2API.GET("incidents", v2.GetIncidentsHandler(a.db, a.log)) + v2API.GET("incidents", cache.GinMiddleware(a.eventsCache), v2.GetIncidentsHandler(a.db, a.log)) v2API.POST("incidents", AuthenticationMW(a.oa2Prov, a.log, a.secretKeyV1, a.authGroup), ValidateComponentsMW(a.db, a.log), + cache.Invalidator(a.eventsCache), v2.PostIncidentHandler(a.db, a.log), ) - v2API.GET("incidents/:eventID", v2.GetIncidentHandler(a.db, a.log)) + v2API.GET("incidents/:eventID", cache.GinMiddleware(a.eventsCache), v2.GetIncidentHandler(a.db, a.log)) v2API.PATCH("incidents/:eventID", AuthenticationMW(a.oa2Prov, a.log, a.secretKeyV1, a.authGroup), CheckEventExistenceMW(a.db, a.log), + cache.Invalidator(a.eventsCache), v2.PatchIncidentHandler(a.db, a.log)) v2API.POST("incidents/:eventID/extract", AuthenticationMW(a.oa2Prov, a.log, a.secretKeyV1, a.authGroup), CheckEventExistenceMW(a.db, a.log), ValidateComponentsMW(a.db, a.log), + cache.Invalidator(a.eventsCache), v2.PostIncidentExtractHandler(a.db, a.log)) v2API.PATCH("incidents/:eventID/updates/:updateID", AuthenticationMW(a.oa2Prov, a.log, a.secretKeyV1, a.authGroup), CheckEventExistenceMW(a.db, a.log), + cache.Invalidator(a.eventsCache), v2.PatchEventUpdateTextHandler(a.db, a.log)) // Events section. // Get /v2/events returns events page with pagination. - v2API.GET("events", v2.GetEventsHandler(a.db, a.log)) + v2API.GET("events", cache.GinMiddleware(a.eventsCache), v2.GetEventsHandler(a.db, a.log)) v2API.POST("events", AuthenticationMW(a.oa2Prov, a.log, a.secretKeyV1, a.authGroup), ValidateComponentsMW(a.db, a.log), + cache.Invalidator(a.eventsCache), v2.PostIncidentHandler(a.db, a.log)) v2API.GET("events/:eventID", - v2.GetIncidentHandler(a.db, a.log)) + cache.GinMiddleware(a.eventsCache), v2.GetIncidentHandler(a.db, a.log)) v2API.PATCH("events/:eventID", AuthenticationMW(a.oa2Prov, a.log, a.secretKeyV1, a.authGroup), CheckEventExistenceMW(a.db, a.log), + cache.Invalidator(a.eventsCache), v2.PatchIncidentHandler(a.db, a.log)) v2API.POST("events/:eventID/extract", AuthenticationMW(a.oa2Prov, a.log, a.secretKeyV1, a.authGroup), CheckEventExistenceMW(a.db, a.log), ValidateComponentsMW(a.db, a.log), + cache.Invalidator(a.eventsCache), v2.PostIncidentExtractHandler(a.db, a.log)) v2API.PATCH("events/:eventID/updates/:updateID", AuthenticationMW(a.oa2Prov, a.log, a.secretKeyV1, a.authGroup), CheckEventExistenceMW(a.db, a.log), + cache.Invalidator(a.eventsCache), v2.PatchEventUpdateTextHandler(a.db, a.log)) // Availability section. - v2API.GET("availability", v2.GetComponentsAvailabilityHandler(a.db, a.log)) + v2API.GET("availability", cache.GinMiddleware(a.componentsCache), v2.GetComponentsAvailabilityHandler(a.db, a.log)) // For testing purposes only. v2API.GET("rss/", newRSS.HandleRSS(a.db, a.log)) diff --git a/internal/cache/cache.go b/internal/cache/cache.go new file mode 100644 index 0000000..fc8dd25 --- /dev/null +++ b/internal/cache/cache.go @@ -0,0 +1,87 @@ +package cache + +import ( + "sync" + "time" +) + +type entry[V any] struct { + value V + expiresAt time.Time +} + +type Cache[V any] struct { + mu sync.RWMutex + items map[string]entry[V] + ttl time.Duration + done chan struct{} +} + +func New[V any](ttl time.Duration) *Cache[V] { + c := &Cache[V]{ + items: make(map[string]entry[V]), + ttl: ttl, + done: make(chan struct{}), + } + go c.janitor() + return c +} + +func (c *Cache[V]) janitor() { + ticker := time.NewTicker(c.ttl) + defer ticker.Stop() + for { + select { + case <-ticker.C: + c.mu.Lock() + now := time.Now() + for k, e := range c.items { + if now.After(e.expiresAt) { + delete(c.items, k) + } + } + c.mu.Unlock() + case <-c.done: + return + } + } +} + +func (c *Cache[V]) Get(key string) (V, bool) { + c.mu.RLock() + e, ok := c.items[key] + c.mu.RUnlock() + + if !ok { + var zero V + return zero, false + } + + if time.Now().After(e.expiresAt) { + var zero V + return zero, false + } + return e.value, true +} + +func (c *Cache[V]) Set(key string, value V) { + c.mu.Lock() + c.items[key] = entry[V]{value: value, expiresAt: time.Now().Add(c.ttl)} + c.mu.Unlock() +} + +func (c *Cache[V]) Invalidate(key string) { + c.mu.Lock() + delete(c.items, key) + c.mu.Unlock() +} + +func (c *Cache[V]) InvalidateAll() { + c.mu.Lock() + c.items = make(map[string]entry[V]) + c.mu.Unlock() +} + +func (c *Cache[V]) Close() { + close(c.done) +} diff --git a/internal/cache/middleware.go b/internal/cache/middleware.go new file mode 100644 index 0000000..8907687 --- /dev/null +++ b/internal/cache/middleware.go @@ -0,0 +1,77 @@ +package cache + +import ( + "bytes" + "net/http" + "time" + + "github.com/gin-gonic/gin" +) + +type CachedResponse struct { + status int + header http.Header + body []byte +} + +// GinMiddleware returns a gin middleware that caches successful GET responses. +func GinMiddleware(c *Cache[CachedResponse]) gin.HandlerFunc { + return func(ctx *gin.Context) { + if ctx.Request.Method != http.MethodGet { + ctx.Next() + return + } + + key := ctx.Request.RequestURI + + if cached, ok := c.Get(key); ok { + for k, vals := range cached.header { + ctx.Writer.Header()[k] = append([]string(nil), vals...) + } + ctx.Writer.Header().Set("X-Cache", "HIT") + ctx.Writer.WriteHeader(cached.status) + _, _ = ctx.Writer.Write(cached.body) + ctx.Abort() + return + } + + w := &responseRecorder{ResponseWriter: ctx.Writer, body: &bytes.Buffer{}} + ctx.Writer = w + + ctx.Next() + + if w.Status() >= 200 && w.Status() < 300 { + c.Set(key, CachedResponse{ + status: w.Status(), + header: w.Header().Clone(), + body: append([]byte(nil), w.body.Bytes()...), + }) + } + } +} + +type responseRecorder struct { + gin.ResponseWriter + body *bytes.Buffer +} + +func (r *responseRecorder) Write(b []byte) (int, error) { + r.body.Write(b) + return r.ResponseWriter.Write(b) +} + +// Invalidator returns a middleware that invalidates the given cache on mutating requests. +func Invalidator(c *Cache[CachedResponse]) gin.HandlerFunc { + return func(ctx *gin.Context) { + ctx.Next() + + if ctx.Request.Method != http.MethodGet && ctx.Writer.Status() < 400 { + c.InvalidateAll() + } + } +} + +// NewHTTPCache creates a cache instance for HTTP responses with the given TTL. +func NewHTTPCache(ttl time.Duration) *Cache[CachedResponse] { + return New[CachedResponse](ttl) +} diff --git a/internal/cache/middleware_test.go b/internal/cache/middleware_test.go new file mode 100644 index 0000000..b1c5275 --- /dev/null +++ b/internal/cache/middleware_test.go @@ -0,0 +1,238 @@ +package cache + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCache(t *testing.T) { + tests := []struct { + name string + run func(t *testing.T) + }{ + { + name: "operations", + run: func(t *testing.T) { + c := New[string](time.Minute) + defer c.Close() + c.Set("key", "value") + + value, ok := c.Get("key") + require.True(t, ok) + assert.Equal(t, "value", value) + + c.Invalidate("key") + _, ok = c.Get("key") + assert.False(t, ok) + + c.Set("one", "1") + c.Set("two", "2") + c.InvalidateAll() + assert.Empty(t, c.items) + + httpCache := NewHTTPCache(time.Second) + defer httpCache.Close() + require.NotNil(t, httpCache) + assert.Equal(t, time.Second, httpCache.ttl) + }, + }, + { + name: "get returns false on expired entry", + run: func(t *testing.T) { + c := New[string](time.Minute) + defer c.Close() + + c.mu.Lock() + c.items["expired"] = entry[string]{ + value: "stale", + expiresAt: time.Now().Add(-time.Second), + } + c.mu.Unlock() + + _, ok := c.Get("expired") + require.False(t, ok) + }, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + tc.run(t) + }) + } +} + +func TestGinMiddleware(t *testing.T) { + gin.SetMode(gin.TestMode) + + tests := []struct { + name string + method string + path string + setupRouter func(r *gin.Engine, cached *Cache[CachedResponse]) + requests int + expectedCode int + expectedBody string + expectedCache []string + }{ + { + name: "caches successful GET response", + method: http.MethodGet, + path: "/cacheable", + setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + reads := 0 + r.GET("/cacheable", GinMiddleware(cached), func(c *gin.Context) { + reads++ + c.Header("X-Single", "value") + c.Writer.Header().Add("X-Multi", "first") + c.Writer.Header().Add("X-Multi", "second") + c.String(http.StatusAccepted, fmt.Sprintf("payload:%d", reads)) + }) + }, + requests: 2, + expectedCode: http.StatusAccepted, + expectedBody: "payload:1", + expectedCache: []string{"", "HIT"}, + }, + { + name: "skips non-GET requests", + method: http.MethodPost, + path: "/resource", + setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + posts := 0 + r.POST("/resource", GinMiddleware(cached), func(c *gin.Context) { + posts++ + c.String(http.StatusCreated, fmt.Sprintf("created:%d", posts)) + }) + }, + requests: 2, + expectedCode: http.StatusCreated, + expectedBody: "created:2", + expectedCache: []string{"", ""}, + }, + { + name: "does not cache non-successful responses", + method: http.MethodGet, + path: "/error", + setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + reads := 0 + r.GET("/error", GinMiddleware(cached), func(c *gin.Context) { + reads++ + c.String(http.StatusInternalServerError, fmt.Sprintf("error:%d", reads)) + }) + }, + requests: 2, + expectedCode: http.StatusInternalServerError, + expectedBody: "error:2", + expectedCache: []string{"", ""}, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cached := NewHTTPCache(time.Minute) + defer cached.Close() + router := gin.New() + tc.setupRouter(router, cached) + + var lastResp *httptest.ResponseRecorder + for i := 0; i < tc.requests; i++ { + lastResp = performRequest(t, router, tc.method, tc.path) + expectedCacheHeader := tc.expectedCache[i] + assert.Equal(t, expectedCacheHeader, lastResp.Header().Get("X-Cache")) + } + + require.Equal(t, tc.expectedCode, lastResp.Code) + assert.Equal(t, tc.expectedBody, lastResp.Body.String()) + }) + } +} + +func TestInvalidator(t *testing.T) { + gin.SetMode(gin.TestMode) + + tests := []struct { + name string + setupRouter func(r *gin.Engine, cached *Cache[CachedResponse]) + actions func(t *testing.T, router *gin.Engine) + }{ + { + name: "ignores failed mutations and GET requests", + setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + reads := 0 + r.GET("/resource", GinMiddleware(cached), Invalidator(cached), func(c *gin.Context) { + reads++ + c.String(http.StatusOK, fmt.Sprintf("resource:%d", reads)) + }) + r.POST("/resource", Invalidator(cached), func(c *gin.Context) { + c.String(http.StatusBadRequest, "bad request") + }) + }, + actions: func(t *testing.T, router *gin.Engine) { + resp := performRequest(t, router, http.MethodGet, "/resource") + require.Equal(t, http.StatusOK, resp.Code) + + resp = performRequest(t, router, http.MethodGet, "/resource") + assert.Equal(t, "HIT", resp.Header().Get("X-Cache")) + + performRequest(t, router, http.MethodPost, "/resource") + + resp = performRequest(t, router, http.MethodGet, "/resource") + assert.Equal(t, "HIT", resp.Header().Get("X-Cache")) + }, + }, + { + name: "invalidates on successful mutation", + setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + reads := 0 + r.GET("/resource", GinMiddleware(cached), func(c *gin.Context) { + reads++ + c.String(http.StatusOK, fmt.Sprintf("resource:%d", reads)) + }) + r.POST("/resource", Invalidator(cached), func(c *gin.Context) { + c.Status(http.StatusCreated) + }) + }, + actions: func(t *testing.T, router *gin.Engine) { + performRequest(t, router, http.MethodGet, "/resource") + performRequest(t, router, http.MethodPost, "/resource") + + resp := performRequest(t, router, http.MethodGet, "/resource") + assert.Empty(t, resp.Header().Get("X-Cache")) + assert.Equal(t, "resource:2", resp.Body.String()) + }, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cached := NewHTTPCache(time.Minute) + defer cached.Close() + router := gin.New() + tc.setupRouter(router, cached) + tc.actions(t, router) + }) + } +} + +func performRequest(t *testing.T, router http.Handler, method, path string) *httptest.ResponseRecorder { + t.Helper() + + req := httptest.NewRequest(method, path, nil) + resp := httptest.NewRecorder() + router.ServeHTTP(resp, req) + return resp +} diff --git a/internal/db/db.go b/internal/db/db.go index cf0c2de..ff0820b 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -15,14 +15,6 @@ import ( "github.com/stackmon/otc-status-dashboard/internal/event" ) -// Connection pool defaults. -const ( - dbMaxOpenConns = 25 - dbMaxIdleConns = 10 - dbConnMaxLifetime = 5 * time.Minute - dbConnMaxIdleTime = 30 * time.Second -) - type DB struct { g *gorm.DB } @@ -48,16 +40,6 @@ func New(c *conf.Config) (*DB, error) { return nil, err } - sqlDB, err := g.DB() - if err != nil { - return nil, fmt.Errorf("getting underlying sql.DB: %w", err) - } - - sqlDB.SetMaxOpenConns(dbMaxOpenConns) - sqlDB.SetMaxIdleConns(dbMaxIdleConns) - sqlDB.SetConnMaxLifetime(dbConnMaxLifetime) - sqlDB.SetConnMaxIdleTime(dbConnMaxIdleTime) - return &DB{g: g}, nil } diff --git a/tests/cache_test.go b/tests/cache_test.go new file mode 100644 index 0000000..f8df843 --- /dev/null +++ b/tests/cache_test.go @@ -0,0 +1,161 @@ +package tests + +import ( + "bytes" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/stackmon/otc-status-dashboard/internal/api" + apiErrors "github.com/stackmon/otc-status-dashboard/internal/api/errors" + v1 "github.com/stackmon/otc-status-dashboard/internal/api/v1" + v2 "github.com/stackmon/otc-status-dashboard/internal/api/v2" + "github.com/stackmon/otc-status-dashboard/internal/cache" + "github.com/stackmon/otc-status-dashboard/internal/conf" + "github.com/stackmon/otc-status-dashboard/internal/db" +) + +func initTestsWithCache(t *testing.T) (r *gin.Engine, componentsCache, eventsCache *cache.Cache[cache.CachedResponse]) { + t.Helper() + + d, err := db.New(&conf.Config{DB: databaseURL}) + require.NoError(t, err) + + componentsCache = cache.NewHTTPCache(5 * time.Second) + eventsCache = cache.NewHTTPCache(5 * time.Second) + + gin.SetMode(gin.TestMode) + r = gin.New() + r.NoRoute(apiErrors.Return404) + r.Use(api.ErrorHandle()) + + logger, _ := zap.NewDevelopment() + + v1API := r.Group("v1") + { + v1API.GET("component_status", cache.GinMiddleware(componentsCache), v1.GetComponentsStatusHandler(d, logger)) + v1API.POST("component_status", cache.Invalidator(componentsCache), v1.PostComponentStatusHandler(d, logger)) + v1API.GET("incidents", cache.GinMiddleware(eventsCache), v1.GetIncidentsHandler(d, logger)) + } + + v2API := r.Group("v2") + { + v2API.GET("components", cache.GinMiddleware(componentsCache), v2.GetComponentsHandler(d, logger)) + v2API.GET("incidents", cache.GinMiddleware(eventsCache), v2.GetIncidentsHandler(d, logger)) + v2API.POST("incidents", api.ValidateComponentsMW(d, logger), cache.Invalidator(eventsCache), v2.PostIncidentHandler(d, logger)) + } + + return r, componentsCache, eventsCache +} + +func TestCacheGETHitOnSecondRequest(t *testing.T) { + tests := []struct { + name string + endpoint string + }{ + {"v2 components", "/v2/components"}, + {"v2 incidents", "/v2/incidents"}, + {"v1 component_status", "/v1/component_status"}, + {"v1 incidents", "/v1/incidents"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + r, cCache, eCache := initTestsWithCache(t) + defer cCache.Close() + defer eCache.Close() + + w1 := httptest.NewRecorder() + req1, _ := http.NewRequest(http.MethodGet, tc.endpoint, nil) + r.ServeHTTP(w1, req1) + require.Equal(t, http.StatusOK, w1.Code) + assert.Empty(t, w1.Header().Get("X-Cache"), "first GET must be a cache MISS") + + w2 := httptest.NewRecorder() + req2, _ := http.NewRequest(http.MethodGet, tc.endpoint, nil) + r.ServeHTTP(w2, req2) + require.Equal(t, http.StatusOK, w2.Code) + assert.Equal(t, "HIT", w2.Header().Get("X-Cache"), "second GET must be a cache HIT") + assert.Equal(t, w1.Body.String(), w2.Body.String(), "cached response body must match original") + }) + } +} + +func TestCacheInvalidation(t *testing.T) { + tests := []struct { + name string + primeEndpoint string + mutateMethod string + mutateEndpoint string + mutateBody string + checkEndpoint string + expectedCache string + }{ + { + name: "successful POST invalidates associated cache", + primeEndpoint: "/v2/incidents", + mutateMethod: http.MethodPost, + mutateEndpoint: "/v2/incidents", + mutateBody: `{"title": "test", "impact": 1, "components": [1], "start_date": "2026-01-01T00:00:00Z", "system": false, "type": "incident"}`, + checkEndpoint: "/v2/incidents", + expectedCache: "", + }, + { + name: "POST to components invalidates components cache", + primeEndpoint: "/v2/components", + mutateMethod: http.MethodPost, + mutateEndpoint: "/v1/component_status", + mutateBody: `{"status": "degraded", "component_id": 1}`, + checkEndpoint: "/v2/components", + expectedCache: "", + }, + { + name: "POST to incidents does NOT invalidate components cache", + primeEndpoint: "/v2/components", + mutateMethod: http.MethodPost, + mutateEndpoint: "/v2/incidents", + mutateBody: `{"title": "test", "impact": 1, "components": [1], "start_date": "2026-01-01T00:00:00Z", "system": false, "type": "incident"}`, + checkEndpoint: "/v2/components", + expectedCache: "HIT", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + r, cCache, eCache := initTestsWithCache(t) + defer cCache.Close() + defer eCache.Close() + + prime := httptest.NewRecorder() + r.ServeHTTP(prime, makeGET(t, tc.primeEndpoint)) + require.Equal(t, http.StatusOK, prime.Code) + + warm := httptest.NewRecorder() + r.ServeHTTP(warm, makeGET(t, tc.primeEndpoint)) + assert.Equal(t, "HIT", warm.Header().Get("X-Cache")) + + postReq, _ := http.NewRequest(tc.mutateMethod, tc.mutateEndpoint, bytes.NewReader([]byte(tc.mutateBody))) + postReq.Header.Set("Content-Type", "application/json") + postW := httptest.NewRecorder() + r.ServeHTTP(postW, postReq) + require.Equal(t, http.StatusOK, postW.Code) + + afterMut := httptest.NewRecorder() + r.ServeHTTP(afterMut, makeGET(t, tc.checkEndpoint)) + assert.Equal(t, tc.expectedCache, afterMut.Header().Get("X-Cache")) + }) + } +} + +func makeGET(t *testing.T, path string) *http.Request { + t.Helper() + req, err := http.NewRequest(http.MethodGet, path, nil) + require.NoError(t, err) + return req +} From 7b1f231ab122608e391e424fb9d44024c390d87e Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Thu, 7 May 2026 19:45:09 +0200 Subject: [PATCH 02/14] cache: refactoring_01 --- internal/api/api.go | 5 +++++ internal/cache/cache.go | 30 ++++++++++++++++++++++-------- internal/cache/middleware.go | 9 ++++++++- internal/cache/middleware_test.go | 4 ++-- tests/cache_test.go | 2 +- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index 11e7259..957e9d1 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -65,6 +65,11 @@ func New(cfg *conf.Config, log *zap.Logger, database *db.DB) (*API, error) { return a, nil } +func (a *API) Close() { + a.componentsCache.Close() + a.eventsCache.Close() +} + func (a *API) Router() *gin.Engine { return a.r } diff --git a/internal/cache/cache.go b/internal/cache/cache.go index fc8dd25..126b048 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -11,17 +11,19 @@ type entry[V any] struct { } type Cache[V any] struct { - mu sync.RWMutex - items map[string]entry[V] - ttl time.Duration - done chan struct{} + mu sync.RWMutex + items map[string]entry[V] + ttl time.Duration + maxItems int + done chan struct{} } -func New[V any](ttl time.Duration) *Cache[V] { +func New[V any](ttl time.Duration, maxItems int) *Cache[V] { c := &Cache[V]{ - items: make(map[string]entry[V]), - ttl: ttl, - done: make(chan struct{}), + items: make(map[string]entry[V]), + ttl: ttl, + maxItems: maxItems, + done: make(chan struct{}), } go c.janitor() return c @@ -66,6 +68,18 @@ func (c *Cache[V]) Get(key string) (V, bool) { func (c *Cache[V]) Set(key string, value V) { c.mu.Lock() + if c.maxItems > 0 && len(c.items) >= c.maxItems { + // Evict the entry closest to expiration. + var oldestKey string + var oldestTime time.Time + for k, e := range c.items { + if oldestTime.IsZero() || e.expiresAt.Before(oldestTime) { + oldestKey = k + oldestTime = e.expiresAt + } + } + delete(c.items, oldestKey) + } c.items[key] = entry[V]{value: value, expiresAt: time.Now().Add(c.ttl)} c.mu.Unlock() } diff --git a/internal/cache/middleware.go b/internal/cache/middleware.go index 8907687..4ed56b5 100644 --- a/internal/cache/middleware.go +++ b/internal/cache/middleware.go @@ -60,6 +60,11 @@ func (r *responseRecorder) Write(b []byte) (int, error) { return r.ResponseWriter.Write(b) } +func (r *responseRecorder) WriteString(s string) (int, error) { + r.body.WriteString(s) + return r.ResponseWriter.WriteString(s) +} + // Invalidator returns a middleware that invalidates the given cache on mutating requests. func Invalidator(c *Cache[CachedResponse]) gin.HandlerFunc { return func(ctx *gin.Context) { @@ -71,7 +76,9 @@ func Invalidator(c *Cache[CachedResponse]) gin.HandlerFunc { } } +const defaultMaxHTTPCacheItems = 1000 + // NewHTTPCache creates a cache instance for HTTP responses with the given TTL. func NewHTTPCache(ttl time.Duration) *Cache[CachedResponse] { - return New[CachedResponse](ttl) + return New[CachedResponse](ttl, defaultMaxHTTPCacheItems) } diff --git a/internal/cache/middleware_test.go b/internal/cache/middleware_test.go index b1c5275..ed0db6b 100644 --- a/internal/cache/middleware_test.go +++ b/internal/cache/middleware_test.go @@ -20,7 +20,7 @@ func TestCache(t *testing.T) { { name: "operations", run: func(t *testing.T) { - c := New[string](time.Minute) + c := New[string](time.Minute, 100) defer c.Close() c.Set("key", "value") @@ -46,7 +46,7 @@ func TestCache(t *testing.T) { { name: "get returns false on expired entry", run: func(t *testing.T) { - c := New[string](time.Minute) + c := New[string](time.Minute, 100) defer c.Close() c.mu.Lock() diff --git a/tests/cache_test.go b/tests/cache_test.go index f8df843..8f2a6e3 100644 --- a/tests/cache_test.go +++ b/tests/cache_test.go @@ -144,7 +144,7 @@ func TestCacheInvalidation(t *testing.T) { postReq.Header.Set("Content-Type", "application/json") postW := httptest.NewRecorder() r.ServeHTTP(postW, postReq) - require.Equal(t, http.StatusOK, postW.Code) + require.Less(t, postW.Code, 400, "mutation request must succeed") afterMut := httptest.NewRecorder() r.ServeHTTP(afterMut, makeGET(t, tc.checkEndpoint)) From 9f7c29ae7685bf6ae8345439e8dd9e3892712cb8 Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Thu, 7 May 2026 19:49:43 +0200 Subject: [PATCH 03/14] O(N) -> O(1) optimization --- internal/cache/cache.go | 51 ++++++++++++++++++++----------- internal/cache/middleware_test.go | 5 ++- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 126b048..a08d5bd 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -1,18 +1,21 @@ package cache import ( + "container/list" "sync" "time" ) type entry[V any] struct { + key string value V expiresAt time.Time } type Cache[V any] struct { mu sync.RWMutex - items map[string]entry[V] + items map[string]*list.Element + order *list.List ttl time.Duration maxItems int done chan struct{} @@ -20,7 +23,8 @@ type Cache[V any] struct { func New[V any](ttl time.Duration, maxItems int) *Cache[V] { c := &Cache[V]{ - items: make(map[string]entry[V]), + items: make(map[string]*list.Element), + order: list.New(), ttl: ttl, maxItems: maxItems, done: make(chan struct{}), @@ -37,9 +41,15 @@ func (c *Cache[V]) janitor() { case <-ticker.C: c.mu.Lock() now := time.Now() - for k, e := range c.items { + for el := c.order.Front(); el != nil; { + e := el.Value.(*entry[V]) if now.After(e.expiresAt) { - delete(c.items, k) + next := el.Next() + c.order.Remove(el) + delete(c.items, e.key) + el = next + } else { + break } } c.mu.Unlock() @@ -51,7 +61,7 @@ func (c *Cache[V]) janitor() { func (c *Cache[V]) Get(key string) (V, bool) { c.mu.RLock() - e, ok := c.items[key] + el, ok := c.items[key] c.mu.RUnlock() if !ok { @@ -59,6 +69,7 @@ func (c *Cache[V]) Get(key string) (V, bool) { return zero, false } + e := el.Value.(*entry[V]) if time.Now().After(e.expiresAt) { var zero V return zero, false @@ -68,31 +79,35 @@ func (c *Cache[V]) Get(key string) (V, bool) { func (c *Cache[V]) Set(key string, value V) { c.mu.Lock() - if c.maxItems > 0 && len(c.items) >= c.maxItems { - // Evict the entry closest to expiration. - var oldestKey string - var oldestTime time.Time - for k, e := range c.items { - if oldestTime.IsZero() || e.expiresAt.Before(oldestTime) { - oldestKey = k - oldestTime = e.expiresAt - } + if el, exists := c.items[key]; exists { + c.order.Remove(el) + delete(c.items, key) + } else if c.maxItems > 0 && len(c.items) >= c.maxItems { + oldest := c.order.Front() + if oldest != nil { + e := oldest.Value.(*entry[V]) + c.order.Remove(oldest) + delete(c.items, e.key) } - delete(c.items, oldestKey) } - c.items[key] = entry[V]{value: value, expiresAt: time.Now().Add(c.ttl)} + e := &entry[V]{key: key, value: value, expiresAt: time.Now().Add(c.ttl)} + c.items[key] = c.order.PushBack(e) c.mu.Unlock() } func (c *Cache[V]) Invalidate(key string) { c.mu.Lock() - delete(c.items, key) + if el, ok := c.items[key]; ok { + c.order.Remove(el) + delete(c.items, key) + } c.mu.Unlock() } func (c *Cache[V]) InvalidateAll() { c.mu.Lock() - c.items = make(map[string]entry[V]) + c.items = make(map[string]*list.Element) + c.order.Init() c.mu.Unlock() } diff --git a/internal/cache/middleware_test.go b/internal/cache/middleware_test.go index ed0db6b..4fa05a0 100644 --- a/internal/cache/middleware_test.go +++ b/internal/cache/middleware_test.go @@ -36,6 +36,7 @@ func TestCache(t *testing.T) { c.Set("two", "2") c.InvalidateAll() assert.Empty(t, c.items) + assert.Equal(t, 0, c.order.Len()) httpCache := NewHTTPCache(time.Second) defer httpCache.Close() @@ -50,10 +51,12 @@ func TestCache(t *testing.T) { defer c.Close() c.mu.Lock() - c.items["expired"] = entry[string]{ + e := &entry[string]{ + key: "expired", value: "stale", expiresAt: time.Now().Add(-time.Second), } + c.items["expired"] = c.order.PushBack(e) c.mu.Unlock() _, ok := c.Get("expired") From a136328befd6f1019bf867a6854a99fae378ba60 Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Thu, 7 May 2026 19:53:59 +0200 Subject: [PATCH 04/14] load tests added --- internal/cache/middleware_test.go | 122 ++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/internal/cache/middleware_test.go b/internal/cache/middleware_test.go index 4fa05a0..c4406f4 100644 --- a/internal/cache/middleware_test.go +++ b/internal/cache/middleware_test.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "sync" "testing" "time" @@ -239,3 +240,124 @@ func performRequest(t *testing.T, router http.Handler, method, path string) *htt router.ServeHTTP(resp, req) return resp } + +// ---------------------------------------------------- +// Stress / load tests +// ---------------------------------------------------- + +func TestCacheConcurrentStress(t *testing.T) { + const ( + workers = 64 + opsPerWk = 5000 + maxItems = 256 + ) + + c := New[string](time.Minute, maxItems) + defer c.Close() + + var wg sync.WaitGroup + wg.Add(workers) + + for w := 0; w < workers; w++ { + go func(id int) { + defer wg.Done() + for i := 0; i < opsPerWk; i++ { + key := fmt.Sprintf("w%d-k%d", id, i%512) + switch i % 5 { + case 0, 1, 2: + c.Set(key, fmt.Sprintf("val-%d", i)) + case 3: + c.Get(key) + case 4: + c.Invalidate(key) + } + } + }(w) + } + + wg.Wait() + + c.mu.RLock() + itemCount := len(c.items) + listLen := c.order.Len() + c.mu.RUnlock() + + assert.Equal(t, itemCount, listLen, "map size must equal list length") + assert.LessOrEqual(t, itemCount, maxItems, "cache must not exceed maxItems") +} + +func TestCacheEvictionOrder(t *testing.T) { + const maxItems = 3 + + c := New[string](time.Minute, maxItems) + defer c.Close() + + c.Set("a", "1") + c.Set("b", "2") + c.Set("c", "3") + + // Cache is full. Next insert must evict "a" (oldest). + c.Set("d", "4") + + _, ok := c.Get("a") + assert.False(t, ok, "oldest entry 'a' must be evicted") + + v, ok := c.Get("d") + require.True(t, ok) + assert.Equal(t, "4", v) + + // Overwrite "b" — must NOT evict anything extra. + c.Set("b", "updated") + v, ok = c.Get("b") + require.True(t, ok) + assert.Equal(t, "updated", v) + + // "c" and "d" must still be present. + _, ok = c.Get("c") + assert.True(t, ok, "'c' must survive overwrite of 'b'") + _, ok = c.Get("d") + assert.True(t, ok, "'d' must survive overwrite of 'b'") + + c.mu.RLock() + assert.Equal(t, maxItems, len(c.items)) + assert.Equal(t, maxItems, c.order.Len()) + c.mu.RUnlock() +} + +func BenchmarkCacheSetGet(b *testing.B) { + c := New[string](time.Minute, 4096) + defer c.Close() + + b.RunParallel(func(pb *testing.PB) { + i := 0 + for pb.Next() { + key := fmt.Sprintf("key-%d", i%4096) + c.Set(key, "value") + c.Get(key) + i++ + } + }) +} + +func BenchmarkGinMiddlewareCacheHit(b *testing.B) { + gin.SetMode(gin.TestMode) + cached := NewHTTPCache(time.Minute) + defer cached.Close() + + router := gin.New() + router.GET("/bench", GinMiddleware(cached), func(ctx *gin.Context) { + ctx.String(http.StatusOK, "payload") + }) + + req := httptest.NewRequest(http.MethodGet, "/bench", nil) + router.ServeHTTP(httptest.NewRecorder(), req) + + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/bench", nil) + router.ServeHTTP(w, r) + } + }) +} From 3ea41703cbe35d8c0f5209a4437d06fba7b9e339 Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Thu, 7 May 2026 20:01:15 +0200 Subject: [PATCH 05/14] test: migrate stress and eviction unit tests into table-driven structure and optimize benchmark performance --- internal/cache/middleware_test.go | 186 ++++++++++++++++-------------- 1 file changed, 99 insertions(+), 87 deletions(-) diff --git a/internal/cache/middleware_test.go b/internal/cache/middleware_test.go index c4406f4..466d417 100644 --- a/internal/cache/middleware_test.go +++ b/internal/cache/middleware_test.go @@ -64,6 +64,89 @@ func TestCache(t *testing.T) { require.False(t, ok) }, }, + { + name: "concurrent stress", + run: func(t *testing.T) { + const ( + workers = 64 + opsPerWk = 5000 + maxItems = 256 + ) + + c := New[string](time.Minute, maxItems) + defer c.Close() + + var wg sync.WaitGroup + wg.Add(workers) + + for w := 0; w < workers; w++ { + go func(id int) { + defer wg.Done() + for i := 0; i < opsPerWk; i++ { + key := fmt.Sprintf("w%d-k%d", id, i%512) + switch i % 5 { + case 0, 1, 2: + c.Set(key, fmt.Sprintf("val-%d", i)) + case 3: + c.Get(key) + case 4: + c.Invalidate(key) + } + } + }(w) + } + + wg.Wait() + + c.mu.RLock() + itemCount := len(c.items) + listLen := c.order.Len() + c.mu.RUnlock() + + assert.Equal(t, itemCount, listLen, "map size must equal list length") + assert.LessOrEqual(t, itemCount, maxItems, "cache must not exceed maxItems") + }, + }, + { + name: "eviction order is FIFO", + run: func(t *testing.T) { + const maxItems = 3 + + c := New[string](time.Minute, maxItems) + defer c.Close() + + c.Set("a", "1") + c.Set("b", "2") + c.Set("c", "3") + + // Cache is full. Next insert must evict "a" (oldest). + c.Set("d", "4") + + _, ok := c.Get("a") + assert.False(t, ok, "oldest entry 'a' must be evicted") + + v, ok := c.Get("d") + require.True(t, ok) + assert.Equal(t, "4", v) + + // Overwrite "b" — must NOT evict anything extra. + c.Set("b", "updated") + v, ok = c.Get("b") + require.True(t, ok) + assert.Equal(t, "updated", v) + + // "c" and "d" must still be present. + _, ok = c.Get("c") + assert.True(t, ok, "'c' must survive overwrite of 'b'") + _, ok = c.Get("d") + assert.True(t, ok, "'d' must survive overwrite of 'b'") + + c.mu.RLock() + assert.Equal(t, maxItems, len(c.items)) + assert.Equal(t, maxItems, c.order.Len()) + c.mu.RUnlock() + }, + }, } for _, tc := range tests { @@ -241,99 +324,28 @@ func performRequest(t *testing.T, router http.Handler, method, path string) *htt return resp } -// ---------------------------------------------------- -// Stress / load tests -// ---------------------------------------------------- - -func TestCacheConcurrentStress(t *testing.T) { - const ( - workers = 64 - opsPerWk = 5000 - maxItems = 256 - ) +// --------------------------------------------------------------------------- +// Benchmarks +// --------------------------------------------------------------------------- - c := New[string](time.Minute, maxItems) - defer c.Close() +func BenchmarkCacheSetGet(b *testing.B) { + const keyCount = 4096 - var wg sync.WaitGroup - wg.Add(workers) - - for w := 0; w < workers; w++ { - go func(id int) { - defer wg.Done() - for i := 0; i < opsPerWk; i++ { - key := fmt.Sprintf("w%d-k%d", id, i%512) - switch i % 5 { - case 0, 1, 2: - c.Set(key, fmt.Sprintf("val-%d", i)) - case 3: - c.Get(key) - case 4: - c.Invalidate(key) - } - } - }(w) + keys := make([]string, keyCount) + for i := range keys { + keys[i] = fmt.Sprintf("key-%d", i) } - wg.Wait() - - c.mu.RLock() - itemCount := len(c.items) - listLen := c.order.Len() - c.mu.RUnlock() - - assert.Equal(t, itemCount, listLen, "map size must equal list length") - assert.LessOrEqual(t, itemCount, maxItems, "cache must not exceed maxItems") -} - -func TestCacheEvictionOrder(t *testing.T) { - const maxItems = 3 - - c := New[string](time.Minute, maxItems) - defer c.Close() - - c.Set("a", "1") - c.Set("b", "2") - c.Set("c", "3") - - // Cache is full. Next insert must evict "a" (oldest). - c.Set("d", "4") - - _, ok := c.Get("a") - assert.False(t, ok, "oldest entry 'a' must be evicted") - - v, ok := c.Get("d") - require.True(t, ok) - assert.Equal(t, "4", v) - - // Overwrite "b" — must NOT evict anything extra. - c.Set("b", "updated") - v, ok = c.Get("b") - require.True(t, ok) - assert.Equal(t, "updated", v) - - // "c" and "d" must still be present. - _, ok = c.Get("c") - assert.True(t, ok, "'c' must survive overwrite of 'b'") - _, ok = c.Get("d") - assert.True(t, ok, "'d' must survive overwrite of 'b'") - - c.mu.RLock() - assert.Equal(t, maxItems, len(c.items)) - assert.Equal(t, maxItems, c.order.Len()) - c.mu.RUnlock() -} - -func BenchmarkCacheSetGet(b *testing.B) { - c := New[string](time.Minute, 4096) + c := New[string](time.Minute, keyCount) defer c.Close() + b.ResetTimer() b.RunParallel(func(pb *testing.PB) { i := 0 for pb.Next() { - key := fmt.Sprintf("key-%d", i%4096) - c.Set(key, "value") - c.Get(key) + k := keys[i%keyCount] + c.Set(k, "value") + c.Get(k) i++ } }) @@ -349,14 +361,14 @@ func BenchmarkGinMiddlewareCacheHit(b *testing.B) { ctx.String(http.StatusOK, "payload") }) - req := httptest.NewRequest(http.MethodGet, "/bench", nil) - router.ServeHTTP(httptest.NewRecorder(), req) + // Warm the cache. + router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/bench", nil)) b.ResetTimer() b.RunParallel(func(pb *testing.PB) { for pb.Next() { w := httptest.NewRecorder() - r := httptest.NewRequest(http.MethodGet, "/bench", nil) + r, _ := http.NewRequest(http.MethodGet, "/bench", nil) router.ServeHTTP(w, r) } }) From 789a9986ba3af065fabd86506fba42fc2ca6792c Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Fri, 8 May 2026 10:35:43 +0200 Subject: [PATCH 06/14] cache: fix memory bloat and panic safety in singleflight --- internal/api/api.go | 4 +- internal/api/errors/errors.go | 6 +- internal/cache/middleware.go | 156 ++++++++++++++++++++++-------- internal/cache/middleware_test.go | 16 +-- internal/db/db.go | 18 ++++ tests/cache_test.go | 2 +- 6 files changed, 150 insertions(+), 52 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index 957e9d1..1be9cf2 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -26,8 +26,8 @@ type API struct { oa2Prov *auth.Provider secretKeyV1 string authGroup string - componentsCache *cache.Cache[cache.CachedResponse] - eventsCache *cache.Cache[cache.CachedResponse] + componentsCache *cache.HTTPCache + eventsCache *cache.HTTPCache } func New(cfg *conf.Config, log *zap.Logger, database *db.DB) (*API, error) { diff --git a/internal/api/errors/errors.go b/internal/api/errors/errors.go index 657dcf9..ad1bc1b 100644 --- a/internal/api/errors/errors.go +++ b/internal/api/errors/errors.go @@ -2,10 +2,10 @@ package errors import ( "errors" - "fmt" "net/http" "github.com/gin-gonic/gin" + "go.uber.org/zap" ) func ReturnError(err error) error { @@ -28,8 +28,8 @@ func Return404(c *gin.Context) { } func RaiseInternalErr(c *gin.Context, err error) { - intErr := fmt.Errorf("%w: %w", ErrInternalError, err) - c.AbortWithStatusJSON(http.StatusInternalServerError, ReturnError(intErr)) + zap.L().Error("internal server error", zap.Error(err)) + c.AbortWithStatusJSON(http.StatusInternalServerError, ReturnError(ErrInternalError)) } func RaiseBadRequestErr(c *gin.Context, err error) { diff --git a/internal/cache/middleware.go b/internal/cache/middleware.go index 4ed56b5..26d0b4c 100644 --- a/internal/cache/middleware.go +++ b/internal/cache/middleware.go @@ -6,16 +6,40 @@ import ( "time" "github.com/gin-gonic/gin" + "golang.org/x/sync/singleflight" ) +// CachedResponse holds a captured HTTP response ready to be replayed. type CachedResponse struct { status int header http.Header body []byte } -// GinMiddleware returns a gin middleware that caches successful GET responses. -func GinMiddleware(c *Cache[CachedResponse]) gin.HandlerFunc { +const defaultMaxHTTPCacheItems = 1000 + +// HTTPCache wraps Cache[CachedResponse] with request coalescing via singleflight +// to prevent the thundering herd problem on cache misses. +type HTTPCache struct { + c *Cache[CachedResponse] + sfg singleflight.Group +} + +// NewHTTPCache creates an HTTPCache with the given TTL. +func NewHTTPCache(ttl time.Duration) *HTTPCache { + return &HTTPCache{c: New[CachedResponse](ttl, defaultMaxHTTPCacheItems)} +} + +// Close stops the background janitor goroutine. +func (h *HTTPCache) Close() { + h.c.Close() +} + +// GinMiddleware returns a Gin middleware that caches successful GET responses. +// Concurrent requests for the same uncached key are coalesced: only one +// goroutine executes the downstream handler while others wait and receive the +// same result, eliminating thundering-herd bursts on cache expiry. +func GinMiddleware(h *HTTPCache) gin.HandlerFunc { return func(ctx *gin.Context) { if ctx.Request.Method != http.MethodGet { ctx.Next() @@ -24,61 +48,117 @@ func GinMiddleware(c *Cache[CachedResponse]) gin.HandlerFunc { key := ctx.Request.RequestURI - if cached, ok := c.Get(key); ok { - for k, vals := range cached.header { - ctx.Writer.Header()[k] = append([]string(nil), vals...) - } - ctx.Writer.Header().Set("X-Cache", "HIT") - ctx.Writer.WriteHeader(cached.status) - _, _ = ctx.Writer.Write(cached.body) + if cached, ok := h.c.Get(key); ok { + writeCached(ctx, cached, true) ctx.Abort() return } - w := &responseRecorder{ResponseWriter: ctx.Writer, body: &bytes.Buffer{}} - ctx.Writer = w + // Save original writer before singleflight so the leader can restore it. + originalWriter := ctx.Writer + isLeader := false + + val, _, _ := h.sfg.Do(key, func() (interface{}, error) { + isLeader = true + + // Double-check: a previous singleflight call may have just populated the cache. + if cached, ok := h.c.Get(key); ok { + return cached, nil + } + + // Use a drain recorder so the response is captured without being sent yet; + // all coalesced goroutines (including the leader) will write it afterwards. + rec := newDrainRecorder(ctx.Writer) + ctx.Writer = rec + defer func() { ctx.Writer = originalWriter }() + ctx.Next() + + resp := CachedResponse{ + status: rec.status, + header: rec.headers, + body: append([]byte(nil), rec.body.Bytes()...), + } + if rec.status >= 200 && rec.status < 300 { + h.c.Set(key, resp) + } + return resp, nil + }) + + cached, ok := val.(CachedResponse) + if !ok { + return + } + // The leader's response was captured (not sent) inside singleflight.Do. + // Write it now — same code path for leader and followers. + // Followers are marked as HIT because they did not hit the backend. + writeCached(ctx, cached, !isLeader) + ctx.Abort() + } +} +// Invalidator returns a middleware that invalidates the given cache on successful mutating requests. +func Invalidator(h *HTTPCache) gin.HandlerFunc { + return func(ctx *gin.Context) { ctx.Next() - if w.Status() >= 200 && w.Status() < 300 { - c.Set(key, CachedResponse{ - status: w.Status(), - header: w.Header().Clone(), - body: append([]byte(nil), w.body.Bytes()...), - }) + if ctx.Request.Method != http.MethodGet && ctx.Writer.Status() < 400 { + h.c.InvalidateAll() } } } -type responseRecorder struct { - gin.ResponseWriter - body *bytes.Buffer +// writeCached replays a captured response into ctx. isHit sets X-Cache: HIT. +func writeCached(ctx *gin.Context, cached CachedResponse, isHit bool) { + for k, vals := range cached.header { + ctx.Writer.Header()[k] = append([]string(nil), vals...) + } + if isHit { + ctx.Writer.Header().Set("X-Cache", "HIT") + } + ctx.Writer.WriteHeader(cached.status) + _, _ = ctx.Writer.Write(cached.body) } -func (r *responseRecorder) Write(b []byte) (int, error) { - r.body.Write(b) - return r.ResponseWriter.Write(b) +// drainRecorder captures a Gin response (status, headers, body) without +// forwarding writes to the underlying connection. +type drainRecorder struct { + gin.ResponseWriter + body bytes.Buffer + status int + headers http.Header + written bool } -func (r *responseRecorder) WriteString(s string) (int, error) { - r.body.WriteString(s) - return r.ResponseWriter.WriteString(s) +func newDrainRecorder(w gin.ResponseWriter) *drainRecorder { + return &drainRecorder{ + ResponseWriter: w, + status: http.StatusOK, + // Clone current headers so that headers set by earlier middleware + // (e.g. CORS) are visible to the handler and included in the capture. + headers: w.Header().Clone(), + } } -// Invalidator returns a middleware that invalidates the given cache on mutating requests. -func Invalidator(c *Cache[CachedResponse]) gin.HandlerFunc { - return func(ctx *gin.Context) { - ctx.Next() +func (r *drainRecorder) Header() http.Header { return r.headers } +func (r *drainRecorder) Status() int { return r.status } +func (r *drainRecorder) Written() bool { return r.written } +func (r *drainRecorder) Size() int { return r.body.Len() } +func (r *drainRecorder) WriteHeaderNow() {} // prevent premature flush +func (r *drainRecorder) Flush() {} // prevent partial writes to conn - if ctx.Request.Method != http.MethodGet && ctx.Writer.Status() < 400 { - c.InvalidateAll() - } - } +func (r *drainRecorder) WriteHeader(code int) { + r.status = code + r.written = true } -const defaultMaxHTTPCacheItems = 1000 +func (r *drainRecorder) Write(b []byte) (int, error) { + r.written = true + return r.body.Write(b) +} -// NewHTTPCache creates a cache instance for HTTP responses with the given TTL. -func NewHTTPCache(ttl time.Duration) *Cache[CachedResponse] { - return New[CachedResponse](ttl, defaultMaxHTTPCacheItems) +func (r *drainRecorder) WriteString(s string) (int, error) { + r.written = true + return r.body.WriteString(s) } + + diff --git a/internal/cache/middleware_test.go b/internal/cache/middleware_test.go index 466d417..4cef0b6 100644 --- a/internal/cache/middleware_test.go +++ b/internal/cache/middleware_test.go @@ -42,7 +42,7 @@ func TestCache(t *testing.T) { httpCache := NewHTTPCache(time.Second) defer httpCache.Close() require.NotNil(t, httpCache) - assert.Equal(t, time.Second, httpCache.ttl) + assert.Equal(t, time.Second, httpCache.c.ttl) }, }, { @@ -165,7 +165,7 @@ func TestGinMiddleware(t *testing.T) { name string method string path string - setupRouter func(r *gin.Engine, cached *Cache[CachedResponse]) + setupRouter func(r *gin.Engine, cached *HTTPCache) requests int expectedCode int expectedBody string @@ -175,7 +175,7 @@ func TestGinMiddleware(t *testing.T) { name: "caches successful GET response", method: http.MethodGet, path: "/cacheable", - setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + setupRouter: func(r *gin.Engine, cached *HTTPCache) { reads := 0 r.GET("/cacheable", GinMiddleware(cached), func(c *gin.Context) { reads++ @@ -194,7 +194,7 @@ func TestGinMiddleware(t *testing.T) { name: "skips non-GET requests", method: http.MethodPost, path: "/resource", - setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + setupRouter: func(r *gin.Engine, cached *HTTPCache) { posts := 0 r.POST("/resource", GinMiddleware(cached), func(c *gin.Context) { posts++ @@ -210,7 +210,7 @@ func TestGinMiddleware(t *testing.T) { name: "does not cache non-successful responses", method: http.MethodGet, path: "/error", - setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + setupRouter: func(r *gin.Engine, cached *HTTPCache) { reads := 0 r.GET("/error", GinMiddleware(cached), func(c *gin.Context) { reads++ @@ -251,12 +251,12 @@ func TestInvalidator(t *testing.T) { tests := []struct { name string - setupRouter func(r *gin.Engine, cached *Cache[CachedResponse]) + setupRouter func(r *gin.Engine, cached *HTTPCache) actions func(t *testing.T, router *gin.Engine) }{ { name: "ignores failed mutations and GET requests", - setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + setupRouter: func(r *gin.Engine, cached *HTTPCache) { reads := 0 r.GET("/resource", GinMiddleware(cached), Invalidator(cached), func(c *gin.Context) { reads++ @@ -281,7 +281,7 @@ func TestInvalidator(t *testing.T) { }, { name: "invalidates on successful mutation", - setupRouter: func(r *gin.Engine, cached *Cache[CachedResponse]) { + setupRouter: func(r *gin.Engine, cached *HTTPCache) { reads := 0 r.GET("/resource", GinMiddleware(cached), func(c *gin.Context) { reads++ diff --git a/internal/db/db.go b/internal/db/db.go index ff0820b..cf0c2de 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -15,6 +15,14 @@ import ( "github.com/stackmon/otc-status-dashboard/internal/event" ) +// Connection pool defaults. +const ( + dbMaxOpenConns = 25 + dbMaxIdleConns = 10 + dbConnMaxLifetime = 5 * time.Minute + dbConnMaxIdleTime = 30 * time.Second +) + type DB struct { g *gorm.DB } @@ -40,6 +48,16 @@ func New(c *conf.Config) (*DB, error) { return nil, err } + sqlDB, err := g.DB() + if err != nil { + return nil, fmt.Errorf("getting underlying sql.DB: %w", err) + } + + sqlDB.SetMaxOpenConns(dbMaxOpenConns) + sqlDB.SetMaxIdleConns(dbMaxIdleConns) + sqlDB.SetConnMaxLifetime(dbConnMaxLifetime) + sqlDB.SetConnMaxIdleTime(dbConnMaxIdleTime) + return &DB{g: g}, nil } diff --git a/tests/cache_test.go b/tests/cache_test.go index 8f2a6e3..2f14e3f 100644 --- a/tests/cache_test.go +++ b/tests/cache_test.go @@ -21,7 +21,7 @@ import ( "github.com/stackmon/otc-status-dashboard/internal/db" ) -func initTestsWithCache(t *testing.T) (r *gin.Engine, componentsCache, eventsCache *cache.Cache[cache.CachedResponse]) { +func initTestsWithCache(t *testing.T) (r *gin.Engine, componentsCache, eventsCache *cache.HTTPCache) { t.Helper() d, err := db.New(&conf.Config{DB: databaseURL}) From bf3ce6c52679e7124e320ca6e26a77ba8e5265bc Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Fri, 8 May 2026 11:59:00 +0200 Subject: [PATCH 07/14] docs: add HTTP caching documentation with load test results --- docs/caching.md | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ docs/readme.md | 1 + 2 files changed, 111 insertions(+) create mode 100644 docs/caching.md diff --git a/docs/caching.md b/docs/caching.md new file mode 100644 index 0000000..17744d5 --- /dev/null +++ b/docs/caching.md @@ -0,0 +1,110 @@ +# HTTP Response Caching + +## Overview + +The Status Dashboard implements an in-memory HTTP response cache with request coalescing +to reduce database load and improve response latency for read-heavy GET endpoints. + +## Architecture + +``` +Request → GinMiddleware → [Cache HIT?] → yes → replay cached response + ↓ no + [singleflight.Do] → only 1 goroutine executes handler + ↓ + [drainRecorder captures response] + ↓ + [store in cache + respond to all waiters] +``` + +### Components + +| Component | Location | Purpose | +|-----------|----------|---------| +| `Cache[V]` | `internal/cache/cache.go` | Generic LRU cache with TTL and background eviction | +| `HTTPCache` | `internal/cache/middleware.go` | Wraps `Cache[CachedResponse]` + `singleflight.Group` | +| `GinMiddleware` | `internal/cache/middleware.go` | Gin handler that serves/populates cache | +| `Invalidator` | `internal/cache/middleware.go` | Gin handler that flushes cache on mutations | +| `drainRecorder` | `internal/cache/middleware.go` | Captures response without sending to client | + +## Configuration + +Defined in `internal/api/api.go`: + +```go +const ( + componentsCacheTTL = 60 * time.Second // /v2/components, /v1/component_status, /v2/availability + eventsCacheTTL = 10 * time.Second // /v2/incidents, /v2/events, /v1/incidents +) +``` + +| Parameter | Value | Notes | +|-----------|-------|-------| +| Components TTL | 60s | Components change rarely | +| Events TTL | 10s | Events/incidents change more frequently | +| Max items | 1000 | Per cache instance (LRU eviction) | + +## Cache Key Strategy + +The cache key is `ctx.Request.RequestURI` — the full path including query parameters. +This means `/v2/events`, `/v2/events?type=incident`, and `/v2/events?type=maintenance` +are cached independently. + +## Thundering Herd Protection + +When TTL expires and multiple concurrent requests arrive for the same key, +`singleflight.Group` ensures only **one** goroutine executes the database query. +All other goroutines wait and receive the same result without hitting the DB. + +## Cache Invalidation + +`Invalidator` middleware is applied to all mutating endpoints (POST/PATCH/PUT). +On a successful mutation (status < 400), it calls `InvalidateAll()` on the +associated cache instance. + +| Cache Instance | Invalidated By | +|----------------|---------------| +| `componentsCache` | `POST /v1/component_status`, `POST /v2/components` | +| `eventsCache` | `POST /v2/incidents`, `PATCH /v2/incidents/:id`, `POST /v2/events`, etc. | + +## Response Headers + +| Header | Value | Meaning | +|--------|-------|---------| +| `X-Cache: HIT` | Present | Response served from cache | +| (absent) | — | Cache miss; response fetched from DB | + +## Load Test Results + +Testing tool: `wrk 4.2.0`. Target: application with PostgreSQL backend. + +### Warm Cache — 4 threads, 50 connections, 30s + +| Endpoint | RPS | P50 | P99 | Max | Errors | +|----------|-----|-----|-----|-----|--------| +| `GET /v2/components` | 56,287 | 0.6ms | 23ms | 140ms | 0 | +| `GET /v2/incidents` | 38,524 | 0.9ms | 10ms | 45ms | 0 | +| `GET /v2/events` | 46,012 | 0.7ms | 208ms | 635ms | 0 | +| `GET /v2/events?type=incident` | 39,032 | 0.9ms | 39ms | 304ms | 0 | +| `GET /v2/events?type=maintenance` | 38,863 | 0.9ms | 211ms | 544ms | 0 | +| `GET /v2/availability` | 43,462 | 0.8ms | 18ms | 221ms | 0 | +| `GET /v1/component_status` | 30,402 | 1.1ms | 26ms | 244ms | 0 | +| `GET /v1/incidents` | 35,717 | 0.9ms | 26ms | 369ms | 0 | + +### Stress Test — 8 threads, 200 connections, 60s + +| Endpoint | RPS | P50 | P99 | Max | Errors | +|----------|-----|-----|-----|-----|--------| +| `GET /v2/components` | 52,046 | 3.1ms | 101ms | 457ms | 0 | +| `GET /v2/incidents` | 40,293 | 4.0ms | 53ms | 299ms | 0 | +| `GET /v2/events` | 52,419 | 3.0ms | 50ms | 198ms | 0 | +| `GET /v2/events?type=incident` | 52,415 | 3.0ms | 62ms | 333ms | 0 | +| `GET /v2/events?type=maintenance` | 46,269 | 3.4ms | 214ms | 1110ms | 0 | +| `GET /v1/component_status` | 36,731 | 4.4ms | 52ms | 319ms | 0 | +| `GET /v1/incidents` | 41,804 | 3.9ms | 72ms | 410ms | 0 | + +### Key Observations + +- Zero errors and zero timeouts under 200 concurrent connections +- Singleflight eliminates P99 spikes on cache expiry (previously up to 1.12s → now 53ms) +- DB connection pool (`MaxOpenConns=25`) prevents connection exhaustion diff --git a/docs/readme.md b/docs/readme.md index 877cbec..afb0b79 100644 --- a/docs/readme.md +++ b/docs/readme.md @@ -2,6 +2,7 @@ ## Table of contents +- [HTTP Response Caching](./caching.md) - [Incident creation for API V1](./v1/v1_incident_creation.md) - [Components availability V2](./v2/v2_components_availability.md) - [Authentication for FE part](./auth/authentication.md) From fc5bdd15f1e6c9ece3a6c70d4e7990c795127b49 Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Fri, 8 May 2026 12:05:56 +0200 Subject: [PATCH 08/14] docs: add distributed deployment considerations to caching guide --- docs/caching.md | 89 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/docs/caching.md b/docs/caching.md index 17744d5..ab589bf 100644 --- a/docs/caching.md +++ b/docs/caching.md @@ -108,3 +108,92 @@ Testing tool: `wrk 4.2.0`. Target: application with PostgreSQL backend. - Zero errors and zero timeouts under 200 concurrent connections - Singleflight eliminates P99 spikes on cache expiry (previously up to 1.12s → now 53ms) - DB connection pool (`MaxOpenConns=25`) prevents connection exhaustion + +## Distributed Deployment Considerations (Kubernetes) + +> **Current scope**: single-pod deployment. This section documents known limitations +> and mitigation strategies for horizontal scaling. + +### Known Limitations + +| Problem | Impact | Severity | +|---------|--------|----------| +| **Stale data across pods** | Each pod has independent cache; POST on Pod A invalidates only Pod A's cache. Pods B, C serve stale data until local TTL expires. | Medium | +| **Per-pod singleflight** | On TTL expiry, each of N pods sends 1 query to DB simultaneously (N total). With 20 pods → 20 concurrent heavy queries. | Medium | +| **Memory duplication** | Same cached responses stored in every pod: total RAM = O(N × cache_size). | Low | + +### Risk Assessment + +With current TTL values and typical payload sizes: + +- **Max staleness window**: 60s (components), 10s (events) +- **DB peak on expiry**: N pods × 1 query (bounded by singleflight within each pod) +- **Memory overhead per pod**: negligible for JSON payloads (~KB each, max 1000 entries) + +**Conclusion**: For ≤5 pods with 10-60s TTL, the current design is acceptable. +Issues become significant at 10+ pods or when data freshness SLA < TTL. + +### Mitigation Without External Dependencies + +| Strategy | Effect | Tradeoff | +|----------|--------|----------| +| **Staggered TTL (jitter)** | Add ±10% random offset to TTL → prevents synchronized cache expiry across pods | Slightly less predictable staleness | +| **Reduce TTL** | Shorter TTL → smaller inconsistency window | Higher DB load (more frequent misses) | +| **Ingress session affinity** | Sticky sessions → one user always hits same pod → no visible flip-flops | Uneven load distribution | + +### Future Architecture Options + +#### Option A — Redis as L2 Cache + +``` +Request → L1 (in-memory) → miss → L2 (Redis) → miss → DB + ↓ + store in L2 + L1 ← response +``` + +- **Solves**: stale data, memory duplication, thundering herd (single Redis fetch) +- **Cost**: +0.5-2ms network RTT on L1 miss; Redis HA infrastructure (Sentinel/Cluster) +- **Invalidation**: DELETE key from Redis on mutation → all pods miss L1 on next request + +#### Option B — Pub/Sub Broadcast Invalidation + +``` +Pod A receives POST → invalidate local cache → publish event to Redis Pub/Sub + ↓ +Pod B, Pod C subscribers → receive event → InvalidateAll() +``` + +- **Solves**: stale data (near-realtime, ~ms propagation) +- **Does not solve**: thundering herd across pods, memory duplication +- **Cost**: minimal latency impact; requires message broker (Redis Pub/Sub, NATS, RabbitMQ) +- **Graceful degradation**: if broker is down, falls back to current TTL-based expiry + +#### Option C — Distributed Singleflight (Redlock) + +``` +TTL expires → Pod tries SET NX lock_key → success → fetch from DB → store in Redis + → failure → poll Redis until result available +``` + +- **Solves**: thundering herd at cluster level (exactly 1 DB query across all pods) +- **Does not solve**: stale data between invalidation events +- **Cost**: high complexity; Redlock requires 3+ Redis nodes; adds failure modes + +### Decision Matrix + +| Criteria | A: Redis L2 | B: Pub/Sub | C: Dist. Singleflight | +|----------|:-----------:|:----------:|:---------------------:| +| Data consistency | ★★★ | ★★☆ | ★☆☆ | +| Latency impact | ★★☆ | ★★★ | ★★★ | +| Thundering herd fix | ★★★ | ★☆☆ | ★★★ | +| Memory efficiency | ★★★ | ★☆☆ | ★☆☆ | +| Implementation complexity | Medium | Low | High | +| Infra dependency | Redis HA | Pub/Sub broker | 3+ Redis nodes | +| Failure mode | SPOF (without HA) | Graceful | SPOF | + +### Recommended Progression + +1. **Now**: Single-pod deployment — current implementation is optimal +2. **2-5 pods**: Add TTL jitter + session affinity — zero code changes needed +3. **5-10 pods**: Implement Option B (Pub/Sub invalidation) — low effort, high ROI +4. **10+ pods / strict SLA**: Implement Option A (Redis L2) + Option B combined From 6d1a535e7cbb99f851bdd042d732858d8d209b4c Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Fri, 8 May 2026 13:01:38 +0200 Subject: [PATCH 09/14] feat: add support for disabling HTTP caching via configuration and handle nil cache instances in middleware and API lifecycles --- internal/api/api.go | 15 +++++++++++---- internal/cache/middleware.go | 9 +++++++++ internal/conf/conf.go | 4 +++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index 1be9cf2..3ebf1a3 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -53,8 +53,11 @@ func New(cfg *conf.Config, log *zap.Logger, database *db.DB) (*API, error) { r.Use(CORSMiddleware()) r.NoRoute(errors.Return404) - componentsCache := cache.NewHTTPCache(componentsCacheTTL) - eventsCache := cache.NewHTTPCache(eventsCacheTTL) + var componentsCache, eventsCache *cache.HTTPCache + if !cfg.CacheDisabled { + componentsCache = cache.NewHTTPCache(componentsCacheTTL) + eventsCache = cache.NewHTTPCache(eventsCacheTTL) + } a := &API{ r: r, db: database, log: log, oa2Prov: oa2Prov, @@ -66,8 +69,12 @@ func New(cfg *conf.Config, log *zap.Logger, database *db.DB) (*API, error) { } func (a *API) Close() { - a.componentsCache.Close() - a.eventsCache.Close() + if a.componentsCache != nil { + a.componentsCache.Close() + } + if a.eventsCache != nil { + a.eventsCache.Close() + } } func (a *API) Router() *gin.Engine { diff --git a/internal/cache/middleware.go b/internal/cache/middleware.go index 26d0b4c..f1fdf18 100644 --- a/internal/cache/middleware.go +++ b/internal/cache/middleware.go @@ -41,6 +41,10 @@ func (h *HTTPCache) Close() { // same result, eliminating thundering-herd bursts on cache expiry. func GinMiddleware(h *HTTPCache) gin.HandlerFunc { return func(ctx *gin.Context) { + if h == nil { + ctx.Next() + return + } if ctx.Request.Method != http.MethodGet { ctx.Next() return @@ -99,6 +103,11 @@ func GinMiddleware(h *HTTPCache) gin.HandlerFunc { // Invalidator returns a middleware that invalidates the given cache on successful mutating requests. func Invalidator(h *HTTPCache) gin.HandlerFunc { return func(ctx *gin.Context) { + if h == nil { + ctx.Next() + return + } + ctx.Next() if ctx.Request.Method != http.MethodGet && ctx.Writer.Status() < 400 { diff --git a/internal/conf/conf.go b/internal/conf/conf.go index 7c2fe5f..aeb211e 100644 --- a/internal/conf/conf.go +++ b/internal/conf/conf.go @@ -41,6 +41,8 @@ type Config struct { // Web URL for the app // Example: https://web.example.com WebURL string `envconfig:"WEB_URL"` + // Disable cache logic + CacheDisabled bool `envconfig:"CACHE_DISABLED"` // Disable authentication for any reasons it doesn't work with hostname like "*prod*" AuthenticationDisabled bool `envconfig:"AUTHENTICATION_DISABLED"` // Secret key for V1 authentication (deprecated) @@ -210,7 +212,7 @@ func (c *Config) Log(logger *zap.Logger) { logger.Info("Storage and logging configuration", zap.String("db", sanitizeDBString(c.DB)), - // zap.String("cache", c.Cache), + zap.Bool("cache_disabled", c.CacheDisabled), zap.String("log_level", c.LogLevel), ) From bbbadd7fbf276fd3df326cd669e1b49476b996f5 Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Fri, 8 May 2026 13:12:00 +0200 Subject: [PATCH 10/14] docs: add RBAC integration guidelines and test handling for disabled cache middleware --- docs/caching.md | 25 +++++++++++++++++++++ internal/cache/middleware_test.go | 36 +++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/docs/caching.md b/docs/caching.md index ab589bf..7bdbf56 100644 --- a/docs/caching.md +++ b/docs/caching.md @@ -197,3 +197,28 @@ TTL expires → Pod tries SET NX lock_key → success → fetch from DB → stor 2. **2-5 pods**: Add TTL jitter + session affinity — zero code changes needed 3. **5-10 pods**: Implement Option B (Pub/Sub invalidation) — low effort, high ROI 4. **10+ pods / strict SLA**: Implement Option A (Redis L2) + Option B combined + +## RBAC Integration Guidelines + +When merging the `feature/rbac` branch or introducing Role-Based Access Control, strictly observe the following architectural constraints to prevent data leakage and ensure cache consistency. + +### 1. Middleware Chain Order (Critical) + +The order of execution in `routes.go` is paramount. Caching must occur **after** all authentication and authorization checks. + +**Correct Order:** +`[Logger] -> [CORS] -> [Auth] -> [RBAC] -> [Cache] -> [Handler]` + +If `cache.GinMiddleware` is placed before RBAC checks, cached data (potentially containing restricted or administrative fields) could be served to unauthorized users, causing a severe security vulnerability. + +### 2. Cache Key Segmentation + +The current cache key uses `ctx.Request.RequestURI`. If RBAC introduces endpoints where the response payload differs based on the user's role (e.g., an Admin sees extra fields in `GET /v2/components` that a regular user does not), a global URI-based cache will lead to privilege escalation or data suppression. + +**Mitigation:** +Extend the cache key to include the user's role or context if the endpoint serves role-specific data: +`key := fmt.Sprintf("%s:%s", userRole, ctx.Request.RequestURI)` + +### 3. Invalidation of New Mutating Routes + +The RBAC branch introduces several new mutating endpoints (e.g., specific `POST`, `PATCH`, `DELETE` operations for incidents). You must manually attach `cache.Invalidator(a.eventsCache)` or `componentsCache` to all new mutating routes during the merge process. Failure to do so will result in stale data being served after a successful mutation. diff --git a/internal/cache/middleware_test.go b/internal/cache/middleware_test.go index 4cef0b6..76829c5 100644 --- a/internal/cache/middleware_test.go +++ b/internal/cache/middleware_test.go @@ -222,6 +222,22 @@ func TestGinMiddleware(t *testing.T) { expectedBody: "error:2", expectedCache: []string{"", ""}, }, + { + name: "skips caching when cache is disabled (h is nil)", + method: http.MethodGet, + path: "/disabled", + setupRouter: func(r *gin.Engine, cached *HTTPCache) { + reads := 0 + r.GET("/disabled", GinMiddleware(nil), func(c *gin.Context) { + reads++ + c.String(http.StatusOK, fmt.Sprintf("payload:%d", reads)) + }) + }, + requests: 2, + expectedCode: http.StatusOK, + expectedBody: "payload:2", // Since it's not cached, it evaluates twice + expectedCache: []string{"", ""}, + }, } for _, tc := range tests { @@ -300,6 +316,26 @@ func TestInvalidator(t *testing.T) { assert.Equal(t, "resource:2", resp.Body.String()) }, }, + { + name: "skips invalidation when cache is disabled (h is nil)", + setupRouter: func(r *gin.Engine, cached *HTTPCache) { + reads := 0 + r.GET("/resource", GinMiddleware(cached), func(c *gin.Context) { + reads++ + c.String(http.StatusOK, fmt.Sprintf("resource:%d", reads)) + }) + r.POST("/resource", Invalidator(nil), func(c *gin.Context) { + c.Status(http.StatusCreated) + }) + }, + actions: func(t *testing.T, router *gin.Engine) { + performRequest(t, router, http.MethodGet, "/resource") + performRequest(t, router, http.MethodPost, "/resource") + + resp := performRequest(t, router, http.MethodGet, "/resource") + assert.Equal(t, "HIT", resp.Header().Get("X-Cache"), "Cache should not be invalidated because h is nil") + }, + }, } for _, tc := range tests { From 15a09822bdd04430cd9499990f12bf4557acc8ab Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Fri, 8 May 2026 13:34:11 +0200 Subject: [PATCH 11/14] tests: fix test isolation and hardcoded assertions - Add resetIncidentSeed helper to restore DB state after destructive tests - Fix cache invalidation test with valid v1 component_status POST body - Use apiErrors constants instead of hardcoded error strings in v2_events - Add cleanup to TestCacheInvalidation and TestV2PatchEventUpdateHandler - Make TestV2GetIncidentsFilteredHandler fully dynamic (no hardcoded IDs) - Make TestV2GetEventsHandler pagination counts dynamic - Fix TestV2PostIncidentsHandler to use relative incident ID - Fix TestV2CreateComponentAndList to match actual error message --- tests/cache_test.go | 4 +- tests/main_test.go | 41 +++++++- tests/v2_events_test.go | 18 ++-- tests/v2_test.go | 214 +++++++++++++++++++++------------------- 4 files changed, 163 insertions(+), 114 deletions(-) diff --git a/tests/cache_test.go b/tests/cache_test.go index 2f14e3f..55a5439 100644 --- a/tests/cache_test.go +++ b/tests/cache_test.go @@ -88,6 +88,8 @@ func TestCacheGETHitOnSecondRequest(t *testing.T) { } func TestCacheInvalidation(t *testing.T) { + t.Cleanup(func() { resetIncidentSeed(t) }) + tests := []struct { name string primeEndpoint string @@ -111,7 +113,7 @@ func TestCacheInvalidation(t *testing.T) { primeEndpoint: "/v2/components", mutateMethod: http.MethodPost, mutateEndpoint: "/v1/component_status", - mutateBody: `{"status": "degraded", "component_id": 1}`, + mutateBody: `{"name":"Distributed Cache Service","impact":1,"text":"Cache invalidation test","attributes":[{"name":"region","value":"EU-NL"}]}`, checkEndpoint: "/v2/components", expectedCache: "", }, diff --git a/tests/main_test.go b/tests/main_test.go index 930bbba..ec0a0c1 100644 --- a/tests/main_test.go +++ b/tests/main_test.go @@ -213,12 +213,45 @@ func truncateIncidents(t *testing.T) { gormDB, err := gorm.Open(gormpostgres.Open(databaseURL), &gorm.Config{}) require.NoError(t, err, "failed to open gorm connection for truncation") + defer func() { + sqlDB, dbErr := gormDB.DB() + require.NoError(t, dbErr, "failed to get sql.DB from gorm for closing") + require.NoError(t, sqlDB.Close(), "failed to close gorm connection") + }() result := gormDB.Exec("TRUNCATE TABLE incident, incident_status, incident_component_relation RESTART IDENTITY") require.NoError(t, result.Error, "failed to truncate incident tables") +} + +// resetIncidentSeed truncates all incident tables and re-inserts the seed data +// from the test dump (incident ID 1 with its component relation and status). +// Use this to restore the database to its initial state after tests that +// create incidents as side effects. +func resetIncidentSeed(t *testing.T) { + t.Helper() + t.Log("resetting incident tables to seed state") + + gormDB, err := gorm.Open(gormpostgres.Open(databaseURL), &gorm.Config{}) + require.NoError(t, err, "failed to open gorm connection for seed reset") + defer func() { + sqlDB, dbErr := gormDB.DB() + require.NoError(t, dbErr, "failed to get sql.DB for closing") + require.NoError(t, sqlDB.Close(), "failed to close gorm connection") + }() - sqlDB, err := gormDB.DB() - require.NoError(t, err, "failed to get sql.DB from gorm for closing") - err = sqlDB.Close() - require.NoError(t, err, "failed to close gorm connection for truncation") + queries := []string{ + `TRUNCATE TABLE incident, incident_status, incident_component_relation RESTART IDENTITY`, + `INSERT INTO incident (id, text, start_date, end_date, impact, type, system, status) + VALUES (1, 'Closed incident without any update', '2025-05-22 10:12:42', '2025-05-22 11:12:42', 1, 'incident', true, 'resolved')`, + `INSERT INTO incident_component_relation (incident_id, component_id) VALUES (1, 1)`, + `INSERT INTO incident_status (id, incident_id, "timestamp", text, status) + VALUES (1, 1, '2025-05-22 11:12:42.559346', 'close incident', 'resolved')`, + `SELECT setval('incident_id_seq', 1, true)`, + `SELECT setval('incident_status_id_seq', 2, true)`, + } + + for _, q := range queries { + result := gormDB.Exec(q) + require.NoError(t, result.Error, "failed to execute seed query") + } } diff --git a/tests/v2_events_test.go b/tests/v2_events_test.go index 7894c1d..f5b21f3 100644 --- a/tests/v2_events_test.go +++ b/tests/v2_events_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + apiErrors "github.com/stackmon/otc-status-dashboard/internal/api/errors" v2 "github.com/stackmon/otc-status-dashboard/internal/api/v2" "github.com/stackmon/otc-status-dashboard/internal/event" ) @@ -830,6 +831,10 @@ func TestV2GetEventsHandler(t *testing.T) { if totalIncidents%10 != 0 { expectedpages++ } + expectedpages20 := totalIncidents / 20 + if totalIncidents%20 != 0 { + expectedpages20++ + } testCases := []struct { name string @@ -857,7 +862,7 @@ func TestV2GetEventsHandler(t *testing.T) { expectedStatusCode: http.StatusOK, expectedTotal: totalIncidents, expectedPages: expectedpages, - expectedItemsCount: 10, + expectedItemsCount: min(10, totalIncidents), expectedLimit: 10, expectedPage: 1, }, @@ -867,7 +872,7 @@ func TestV2GetEventsHandler(t *testing.T) { expectedStatusCode: http.StatusOK, expectedTotal: totalIncidents, expectedPages: expectedpages, - expectedItemsCount: 10, + expectedItemsCount: max(0, min(10, totalIncidents-10)), expectedLimit: 10, expectedPage: 2, }, @@ -876,8 +881,8 @@ func TestV2GetEventsHandler(t *testing.T) { queryParams: "?limit=20&page=1", expectedStatusCode: http.StatusOK, expectedTotal: totalIncidents, - expectedPages: 2, - expectedItemsCount: 20, + expectedPages: expectedpages20, + expectedItemsCount: min(20, totalIncidents), expectedLimit: 20, expectedPage: 1, }, @@ -1077,6 +1082,7 @@ func TestV2PatchEventUpdateHandler(t *testing.T) { // Clean up database before test to ensure a clean state for this test case. truncateIncidents(t) + t.Cleanup(func() { resetIncidentSeed(t) }) components := []int{1} impact := 1 @@ -1131,7 +1137,7 @@ func TestV2PatchEventUpdateHandler(t *testing.T) { updateIndex: 0, body: `{"text": "This should fail."}`, expectedStatus: http.StatusNotFound, - expectedBody: `{"errMsg":"incident not found"}`, + expectedBody: fmt.Sprintf(`{"errMsg":"%s"}`, apiErrors.ErrIncidentDSNotExist), }, { name: "Update index not found", @@ -1139,7 +1145,7 @@ func TestV2PatchEventUpdateHandler(t *testing.T) { updateIndex: 99, body: `{"text": "This should also fail."}`, expectedStatus: http.StatusNotFound, - expectedBody: `{"errMsg":"update not found"}`, + expectedBody: fmt.Sprintf(`{"errMsg":"%s"}`, apiErrors.ErrUpdateDSNotExist), }, { name: "Invalid update index (negative)", diff --git a/tests/v2_test.go b/tests/v2_test.go index dbc0a72..978b76f 100644 --- a/tests/v2_test.go +++ b/tests/v2_test.go @@ -337,7 +337,7 @@ func TestV2PostIncidentsHandler(t *testing.T) { } result = v2CreateIncident(t, r, &incidentCreateData) require.NotNil(t, result, "v2CreateIncident returned nil") - assert.Equal(t, 23, result.Result[0].IncidentID) + assert.Equal(t, len(incidents)+4, result.Result[0].IncidentID) assert.Equal(t, 3, result.Result[0].ComponentID) } @@ -742,7 +742,7 @@ func TestV2CreateComponentAndList(t *testing.T) { r.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) - assert.Contains(t, w.Body.String(), "component exists") + assert.Contains(t, w.Body.String(), "component already exists") // Test case 3: Try to create component with invalid attributes (duplicate region) t.Log("Test case 3: Create component with invalid attributes") @@ -768,108 +768,118 @@ func TestV2GetIncidentsFilteredHandler(t *testing.T) { t.Log("start to test GET /v2/incidents with filters") r, _, _ := initTests(t) + // Fetch all incidents to compute expected filter results dynamically. + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, v2IncidentsEndpoint, nil) + r.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + + var allResponse V2IncidentsListResponse + err := json.Unmarshal(w.Body.Bytes(), &allResponse) + require.NoError(t, err) + allIncidents := allResponse.Data + require.NotEmpty(t, allIncidents, "Expected incidents in the database") + + collectIDs := func(incidents []*v2.Incident) []int { + ids := make([]int, len(incidents)) + for i, inc := range incidents { + ids[i] = inc.ID + } + return ids + } + + filterIncidents := func(predicate func(*v2.Incident) bool) []int { + var filtered []*v2.Incident + for _, inc := range allIncidents { + if predicate(inc) { + filtered = append(filtered, inc) + } + } + return collectIDs(filtered) + } + + byImpact := func(impact int) func(*v2.Incident) bool { + return func(inc *v2.Incident) bool { + return inc.Impact != nil && *inc.Impact == impact + } + } + bySystem := func(system bool) func(*v2.Incident) bool { + return func(inc *v2.Incident) bool { + return inc.System != nil && *inc.System == system + } + } + byComponent := func(id int) func(*v2.Incident) bool { + return func(inc *v2.Incident) bool { + for _, c := range inc.Components { + if c == id { + return true + } + } + return false + } + } + isActive := func(inc *v2.Incident) bool { + now := time.Now().UTC() + if inc.EndDate == nil { + return true + } + terminal := inc.Status == event.IncidentResolved || + inc.Status == event.MaintenanceCompleted || + inc.Status == event.MaintenanceCancelled || + inc.Status == event.InfoCompleted || + inc.Status == event.InfoCancelled + return !inc.StartDate.After(now) && !inc.EndDate.Before(now) && !terminal + } + afterDate := func(d time.Time) func(*v2.Incident) bool { + return func(inc *v2.Incident) bool { return !inc.StartDate.Before(d) } + } + beforeDate := func(d time.Time) func(*v2.Incident) bool { + return func(inc *v2.Incident) bool { return !inc.StartDate.After(d) } + } + + allIDs := collectIDs(allIncidents) + sd := time.Date(2025, 2, 1, 0, 0, 0, 0, time.UTC) + ed := time.Date(2025, 5, 23, 0, 0, 0, 0, time.UTC) + rangeStart := time.Date(2025, 5, 1, 0, 0, 0, 0, time.UTC) + rangeEnd := time.Date(2025, 5, 24, 0, 0, 0, 0, time.UTC) + type filterTestCase struct { - name string - queryParams map[string]string - expectedIDs []int - expectedCount int + name string + queryParams map[string]string + expectedIDs []int } testCases := []filterTestCase{ - { - name: "No filters", - queryParams: nil, - expectedIDs: []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27}, - expectedCount: 27, - }, - { - name: "Filter by start_date", - queryParams: map[string]string{"start_date": time.Date(2025, 2, 1, 0, 0, 0, 0, time.UTC).Format(time.RFC3339)}, - // Incidents starting on or after 2025-02-01 - expectedIDs: []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27}, - expectedCount: 27, - }, - { - name: "Filter by end_date", - queryParams: map[string]string{"end_date": time.Date(2025, 5, 23, 0, 0, 0, 0, time.UTC).Format(time.RFC3339)}, - // Incidents starting on or before 2025-05-23 - expectedIDs: []int{1}, - expectedCount: 1, - }, - { - name: "Filter by impact minor (1)", - queryParams: map[string]string{"impact": "1"}, - expectedIDs: []int{1, 13, 20, 21, 23, 24, 26, 27}, - expectedCount: 8, - }, - { - name: "Filter by impact major (2)", - queryParams: map[string]string{"impact": "2"}, - expectedIDs: []int{2, 4, 7, 9, 10, 15, 16, 19, 25}, - expectedCount: 9, - }, - { - name: "Filter by impact maintenance (0)", - queryParams: map[string]string{"impact": "0"}, - expectedIDs: []int{6, 8, 17, 22}, - expectedCount: 4, - }, - { - name: "Filter by component_id 1", - queryParams: map[string]string{"components": "1"}, - expectedIDs: []int{1, 5, 22, 24, 25, 26}, - expectedCount: 6, - }, - { - name: "Filter by non-existent component_id 8", - queryParams: map[string]string{"components": "8"}, - expectedIDs: []int{}, - expectedCount: 0, - }, - { - name: "Filter by system true", - queryParams: map[string]string{"system": "true"}, - expectedIDs: []int{1, 7, 10, 11, 12, 13, 14, 15, 16, 18}, - expectedCount: 10, - }, - { - name: "Filter by system false", - queryParams: map[string]string{"system": "false"}, - expectedIDs: []int{2, 3, 4, 5, 6, 8, 9, 17, 19, 20, 21, 22, 23, 24, 25, 26, 27}, - expectedCount: 17, - }, - { - name: "Filter by active true", - queryParams: map[string]string{"active": "true"}, - expectedIDs: []int{26, 27}, - expectedCount: 2, - }, - { - name: "Combination: active true and impact 1", - queryParams: map[string]string{"active": "true", "impact": "1"}, - expectedIDs: []int{26, 27}, - expectedCount: 2, - }, - { - name: "Combination: component_id 3 and system true", - queryParams: map[string]string{"components": "3", "system": "true"}, - expectedIDs: []int{7, 12, 14, 16}, - expectedCount: 4, - }, - { - name: "Date range: 2025-05-01 to 2025-05-24", - queryParams: map[string]string{"start_date": time.Date(2025, 5, 01, 0, 0, 0, 0, time.UTC).Format(time.RFC3339), "end_date": time.Date(2025, 5, 24, 0, 0, 0, 0, time.UTC).Format(time.RFC3339)}, - // Incidents starting between 2025-05-01 and 2025-05-24 (inclusive for start_date) - // No pre-existing incidents in this range. - expectedIDs: []int{1}, - expectedCount: 1, - }, - { - name: "Filter by impact 3 (outage)", - queryParams: map[string]string{"impact": "3"}, - expectedIDs: []int{3, 5, 11, 12, 14, 18}, - expectedCount: 6, - }, + {"No filters", nil, allIDs}, + {"Filter by start_date", map[string]string{"start_date": sd.Format(time.RFC3339)}, + filterIncidents(afterDate(sd))}, + {"Filter by end_date", map[string]string{"end_date": ed.Format(time.RFC3339)}, + filterIncidents(beforeDate(ed))}, + {"Filter by impact minor (1)", map[string]string{"impact": "1"}, + filterIncidents(byImpact(1))}, + {"Filter by impact major (2)", map[string]string{"impact": "2"}, + filterIncidents(byImpact(2))}, + {"Filter by impact maintenance (0)", map[string]string{"impact": "0"}, + filterIncidents(byImpact(0))}, + {"Filter by component_id 1", map[string]string{"components": "1"}, + filterIncidents(byComponent(1))}, + {"Filter by non-existent component_id 8", map[string]string{"components": "8"}, + []int{}}, + {"Filter by system true", map[string]string{"system": "true"}, + filterIncidents(bySystem(true))}, + {"Filter by system false", map[string]string{"system": "false"}, + filterIncidents(bySystem(false))}, + {"Filter by active true", map[string]string{"active": "true"}, + filterIncidents(isActive)}, + {"Combination: active true and impact 1", map[string]string{"active": "true", "impact": "1"}, + filterIncidents(func(inc *v2.Incident) bool { return isActive(inc) && byImpact(1)(inc) })}, + {"Combination: component_id 3 and system true", map[string]string{"components": "3", "system": "true"}, + filterIncidents(func(inc *v2.Incident) bool { return byComponent(3)(inc) && bySystem(true)(inc) })}, + {"Date range: 2025-05-01 to 2025-05-24", + map[string]string{"start_date": rangeStart.Format(time.RFC3339), "end_date": rangeEnd.Format(time.RFC3339)}, + filterIncidents(func(inc *v2.Incident) bool { return afterDate(rangeStart)(inc) && beforeDate(rangeEnd)(inc) })}, + {"Filter by impact 3 (outage)", map[string]string{"impact": "3"}, + filterIncidents(byImpact(3))}, } for _, tc := range testCases { @@ -892,9 +902,7 @@ func TestV2GetIncidentsFilteredHandler(t *testing.T) { require.NoError(t, err, "Failed to unmarshal response for: "+tc.name) actualIncidents := responseData.Data - assert.Len(t, actualIncidents, tc.expectedCount, "Unexpected number of incidents for: "+tc.name) - - // When incidents are found or not, the message field should ideally be empty. + assert.Len(t, actualIncidents, len(tc.expectedIDs), "Unexpected number of incidents for: "+tc.name) assert.Empty(t, responseData.Message, "Expected no message for: "+tc.name) actualIDs := make([]int, len(actualIncidents)) From ceb86dcd01e062c03f0c68c44740ef0f22aafa1d Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Fri, 8 May 2026 15:35:49 +0200 Subject: [PATCH 12/14] refactor: replace incident truncation with centralized resetIncidentSeed helper and standardized t.Cleanup usage --- tests/main_test.go | 49 +++++++++++++++++------------------------ tests/v2_events_test.go | 4 ++-- tests/v2_test.go | 4 ++-- 3 files changed, 24 insertions(+), 33 deletions(-) diff --git a/tests/main_test.go b/tests/main_test.go index ec0a0c1..72ad756 100644 --- a/tests/main_test.go +++ b/tests/main_test.go @@ -207,39 +207,17 @@ func initRoutesV2(t *testing.T, c *gin.Engine, dbInst *db.DB, logger *zap.Logger v2Api.GET("availability", v2.GetComponentsAvailabilityHandler(dbInst, logger)) } -func truncateIncidents(t *testing.T) { - t.Helper() - t.Log("cleaning up incident-related tables before test") - - gormDB, err := gorm.Open(gormpostgres.Open(databaseURL), &gorm.Config{}) - require.NoError(t, err, "failed to open gorm connection for truncation") - defer func() { - sqlDB, dbErr := gormDB.DB() - require.NoError(t, dbErr, "failed to get sql.DB from gorm for closing") - require.NoError(t, sqlDB.Close(), "failed to close gorm connection") - }() - - result := gormDB.Exec("TRUNCATE TABLE incident, incident_status, incident_component_relation RESTART IDENTITY") - require.NoError(t, result.Error, "failed to truncate incident tables") -} - // resetIncidentSeed truncates all incident tables and re-inserts the seed data // from the test dump (incident ID 1 with its component relation and status). // Use this to restore the database to its initial state after tests that -// create incidents as side effects. +// modify incidents. Typically called via t.Cleanup: +// +// t.Cleanup(func() { resetIncidentSeed(t) }) func resetIncidentSeed(t *testing.T) { t.Helper() t.Log("resetting incident tables to seed state") - gormDB, err := gorm.Open(gormpostgres.Open(databaseURL), &gorm.Config{}) - require.NoError(t, err, "failed to open gorm connection for seed reset") - defer func() { - sqlDB, dbErr := gormDB.DB() - require.NoError(t, dbErr, "failed to get sql.DB for closing") - require.NoError(t, sqlDB.Close(), "failed to close gorm connection") - }() - - queries := []string{ + execSQL(t, `TRUNCATE TABLE incident, incident_status, incident_component_relation RESTART IDENTITY`, `INSERT INTO incident (id, text, start_date, end_date, impact, type, system, status) VALUES (1, 'Closed incident without any update', '2025-05-22 10:12:42', '2025-05-22 11:12:42', 1, 'incident', true, 'resolved')`, @@ -248,10 +226,23 @@ func resetIncidentSeed(t *testing.T) { VALUES (1, 1, '2025-05-22 11:12:42.559346', 'close incident', 'resolved')`, `SELECT setval('incident_id_seq', 1, true)`, `SELECT setval('incident_status_id_seq', 2, true)`, - } + ) +} + +// execSQL opens a short-lived gorm connection and executes the given queries +// sequentially inside a single session. Fails the test on any error. +func execSQL(t *testing.T, queries ...string) { + t.Helper() + + gormDB, err := gorm.Open(gormpostgres.Open(databaseURL), &gorm.Config{}) + require.NoError(t, err, "failed to open gorm connection") + defer func() { + sqlDB, dbErr := gormDB.DB() + require.NoError(t, dbErr, "failed to get sql.DB for closing") + require.NoError(t, sqlDB.Close(), "failed to close gorm connection") + }() for _, q := range queries { - result := gormDB.Exec(q) - require.NoError(t, result.Error, "failed to execute seed query") + require.NoError(t, gormDB.Exec(q).Error, "failed to execute query") } } diff --git a/tests/v2_events_test.go b/tests/v2_events_test.go index f5b21f3..5e41d53 100644 --- a/tests/v2_events_test.go +++ b/tests/v2_events_test.go @@ -1080,8 +1080,8 @@ func TestV2PatchEventUpdateHandler(t *testing.T) { t.Log("start to test PATCH /v2/events/:incidentID/updates/:updateID") r, _, _ := initTests(t) - // Clean up database before test to ensure a clean state for this test case. - truncateIncidents(t) + // Reset to seed state before and after test to ensure isolation. + resetIncidentSeed(t) t.Cleanup(func() { resetIncidentSeed(t) }) components := []int{1} diff --git a/tests/v2_test.go b/tests/v2_test.go index 978b76f..6a452c7 100644 --- a/tests/v2_test.go +++ b/tests/v2_test.go @@ -1082,7 +1082,7 @@ func TestV2PostInfoWithExistingEventsHandler(t *testing.T) { } func TestV2GetComponentsAvailability(t *testing.T) { - truncateIncidents(t) + t.Cleanup(func() { resetIncidentSeed(t) }) t.Logf("start to test GET %s", v2AvailabilityEndpoint) r, _, _ := initTests(t) @@ -1191,7 +1191,7 @@ func TestV2PatchIncidentUpdateHandler(t *testing.T) { r, _, _ := initTests(t) // Clean up database before test to ensure a clean state for this test case. - truncateIncidents(t) + t.Cleanup(func() { resetIncidentSeed(t) }) components := []int{1} impact := 1 From 10b7bcc81ca887cde60ab4080717af1634c23d78 Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Fri, 8 May 2026 15:40:50 +0200 Subject: [PATCH 13/14] golangci-lint warnings refactored --- internal/cache/cache.go | 6 +++--- internal/cache/middleware.go | 14 ++++++-------- internal/cache/middleware_test.go | 24 +++++++++++------------- tests/cache_test.go | 8 ++++---- tests/v2_test.go | 16 ++++++++-------- 5 files changed, 32 insertions(+), 36 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index a08d5bd..aa12d9c 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -42,7 +42,7 @@ func (c *Cache[V]) janitor() { c.mu.Lock() now := time.Now() for el := c.order.Front(); el != nil; { - e := el.Value.(*entry[V]) + e, _ := el.Value.(*entry[V]) if now.After(e.expiresAt) { next := el.Next() c.order.Remove(el) @@ -69,7 +69,7 @@ func (c *Cache[V]) Get(key string) (V, bool) { return zero, false } - e := el.Value.(*entry[V]) + e, _ := el.Value.(*entry[V]) if time.Now().After(e.expiresAt) { var zero V return zero, false @@ -85,7 +85,7 @@ func (c *Cache[V]) Set(key string, value V) { } else if c.maxItems > 0 && len(c.items) >= c.maxItems { oldest := c.order.Front() if oldest != nil { - e := oldest.Value.(*entry[V]) + e, _ := oldest.Value.(*entry[V]) c.order.Remove(oldest) delete(c.items, e.key) } diff --git a/internal/cache/middleware.go b/internal/cache/middleware.go index f1fdf18..df1dfa2 100644 --- a/internal/cache/middleware.go +++ b/internal/cache/middleware.go @@ -148,12 +148,12 @@ func newDrainRecorder(w gin.ResponseWriter) *drainRecorder { } } -func (r *drainRecorder) Header() http.Header { return r.headers } -func (r *drainRecorder) Status() int { return r.status } -func (r *drainRecorder) Written() bool { return r.written } -func (r *drainRecorder) Size() int { return r.body.Len() } -func (r *drainRecorder) WriteHeaderNow() {} // prevent premature flush -func (r *drainRecorder) Flush() {} // prevent partial writes to conn +func (r *drainRecorder) Header() http.Header { return r.headers } +func (r *drainRecorder) Status() int { return r.status } +func (r *drainRecorder) Written() bool { return r.written } +func (r *drainRecorder) Size() int { return r.body.Len() } +func (r *drainRecorder) WriteHeaderNow() {} // prevent premature flush +func (r *drainRecorder) Flush() {} // prevent partial writes to conn func (r *drainRecorder) WriteHeader(code int) { r.status = code @@ -169,5 +169,3 @@ func (r *drainRecorder) WriteString(s string) (int, error) { r.written = true return r.body.WriteString(s) } - - diff --git a/internal/cache/middleware_test.go b/internal/cache/middleware_test.go index 76829c5..0297599 100644 --- a/internal/cache/middleware_test.go +++ b/internal/cache/middleware_test.go @@ -14,6 +14,8 @@ import ( ) func TestCache(t *testing.T) { + t.Parallel() + tests := []struct { name string run func(t *testing.T) @@ -79,10 +81,10 @@ func TestCache(t *testing.T) { var wg sync.WaitGroup wg.Add(workers) - for w := 0; w < workers; w++ { + for w := range workers { go func(id int) { defer wg.Done() - for i := 0; i < opsPerWk; i++ { + for i := range opsPerWk { key := fmt.Sprintf("w%d-k%d", id, i%512) switch i % 5 { case 0, 1, 2: @@ -99,12 +101,9 @@ func TestCache(t *testing.T) { wg.Wait() c.mu.RLock() - itemCount := len(c.items) - listLen := c.order.Len() + assert.Len(t, c.items, c.order.Len(), "map size must equal list length") + assert.LessOrEqual(t, len(c.items), maxItems, "cache must not exceed maxItems") c.mu.RUnlock() - - assert.Equal(t, itemCount, listLen, "map size must equal list length") - assert.LessOrEqual(t, itemCount, maxItems, "cache must not exceed maxItems") }, }, { @@ -142,7 +141,7 @@ func TestCache(t *testing.T) { assert.True(t, ok, "'d' must survive overwrite of 'b'") c.mu.RLock() - assert.Equal(t, maxItems, len(c.items)) + assert.Len(t, c.items, maxItems) assert.Equal(t, maxItems, c.order.Len()) c.mu.RUnlock() }, @@ -150,7 +149,6 @@ func TestCache(t *testing.T) { } for _, tc := range tests { - tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() tc.run(t) @@ -159,6 +157,7 @@ func TestCache(t *testing.T) { } func TestGinMiddleware(t *testing.T) { + t.Parallel() gin.SetMode(gin.TestMode) tests := []struct { @@ -226,7 +225,7 @@ func TestGinMiddleware(t *testing.T) { name: "skips caching when cache is disabled (h is nil)", method: http.MethodGet, path: "/disabled", - setupRouter: func(r *gin.Engine, cached *HTTPCache) { + setupRouter: func(r *gin.Engine, _ *HTTPCache) { reads := 0 r.GET("/disabled", GinMiddleware(nil), func(c *gin.Context) { reads++ @@ -241,7 +240,6 @@ func TestGinMiddleware(t *testing.T) { } for _, tc := range tests { - tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() cached := NewHTTPCache(time.Minute) @@ -250,7 +248,7 @@ func TestGinMiddleware(t *testing.T) { tc.setupRouter(router, cached) var lastResp *httptest.ResponseRecorder - for i := 0; i < tc.requests; i++ { + for i := range tc.requests { lastResp = performRequest(t, router, tc.method, tc.path) expectedCacheHeader := tc.expectedCache[i] assert.Equal(t, expectedCacheHeader, lastResp.Header().Get("X-Cache")) @@ -263,6 +261,7 @@ func TestGinMiddleware(t *testing.T) { } func TestInvalidator(t *testing.T) { + t.Parallel() gin.SetMode(gin.TestMode) tests := []struct { @@ -339,7 +338,6 @@ func TestInvalidator(t *testing.T) { } for _, tc := range tests { - tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() cached := NewHTTPCache(time.Minute) diff --git a/tests/cache_test.go b/tests/cache_test.go index 55a5439..fbf3f22 100644 --- a/tests/cache_test.go +++ b/tests/cache_test.go @@ -21,17 +21,17 @@ import ( "github.com/stackmon/otc-status-dashboard/internal/db" ) -func initTestsWithCache(t *testing.T) (r *gin.Engine, componentsCache, eventsCache *cache.HTTPCache) { +func initTestsWithCache(t *testing.T) (*gin.Engine, *cache.HTTPCache, *cache.HTTPCache) { t.Helper() d, err := db.New(&conf.Config{DB: databaseURL}) require.NoError(t, err) - componentsCache = cache.NewHTTPCache(5 * time.Second) - eventsCache = cache.NewHTTPCache(5 * time.Second) + componentsCache := cache.NewHTTPCache(5 * time.Second) + eventsCache := cache.NewHTTPCache(5 * time.Second) gin.SetMode(gin.TestMode) - r = gin.New() + r := gin.New() r.NoRoute(apiErrors.Return404) r.Use(api.ErrorHandle()) diff --git a/tests/v2_test.go b/tests/v2_test.go index 6a452c7..925187a 100644 --- a/tests/v2_test.go +++ b/tests/v2_test.go @@ -884,22 +884,22 @@ func TestV2GetIncidentsFilteredHandler(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - w := httptest.NewRecorder() - req, _ := http.NewRequest(http.MethodGet, v2IncidentsEndpoint, nil) + rec := httptest.NewRecorder() + httpReq, _ := http.NewRequest(http.MethodGet, v2IncidentsEndpoint, nil) - q := req.URL.Query() + q := httpReq.URL.Query() for k, v := range tc.queryParams { q.Add(k, v) } - req.URL.RawQuery = q.Encode() + httpReq.URL.RawQuery = q.Encode() - r.ServeHTTP(w, req) + r.ServeHTTP(rec, httpReq) - assert.Equal(t, http.StatusOK, w.Code, "Unexpected status code for: "+tc.name) + assert.Equal(t, http.StatusOK, rec.Code, "Unexpected status code for: "+tc.name) var responseData V2IncidentsListResponse - err := json.Unmarshal(w.Body.Bytes(), &responseData) - require.NoError(t, err, "Failed to unmarshal response for: "+tc.name) + uerr := json.Unmarshal(rec.Body.Bytes(), &responseData) + require.NoError(t, uerr, "Failed to unmarshal response for: "+tc.name) actualIncidents := responseData.Data assert.Len(t, actualIncidents, len(tc.expectedIDs), "Unexpected number of incidents for: "+tc.name) From 31272be21d3c63f9d16870197d279dc7a1e7779f Mon Sep 17 00:00:00 2001 From: Ilia Bakhterev Date: Fri, 8 May 2026 15:46:03 +0200 Subject: [PATCH 14/14] docs updated --- docker-compose.yaml | 6 ++++++ docs/caching.md | 31 +++++++++++++++++++++++++++++++ internal/conf/conf.go | 5 ++++- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/docker-compose.yaml b/docker-compose.yaml index 1716ee0..e977acd 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -23,6 +23,12 @@ services: volumes: - keycloak:/opt/keycloak/data/ command: start-dev + app: + container_name: app + build: . + environment: + - DB=postgresql://pg:pass@database:5432/status_dashboard + - CACHE_DISABLED=false volumes: db: diff --git a/docs/caching.md b/docs/caching.md index 7bdbf56..9784de9 100644 --- a/docs/caching.md +++ b/docs/caching.md @@ -27,6 +27,37 @@ Request → GinMiddleware → [Cache HIT?] → yes → replay cached response | `Invalidator` | `internal/cache/middleware.go` | Gin handler that flushes cache on mutations | | `drainRecorder` | `internal/cache/middleware.go` | Captures response without sending to client | +## Disabling Cache + +The cache can be fully disabled via the `CACHE_DISABLED` environment variable: + +```bash +export CACHE_DISABLED=true +``` + +When disabled: +- No `HTTPCache` instances are created (`nil`) +- `GinMiddleware` and `Invalidator` become pass-through — every request hits the database directly +- No background janitor goroutines are started +- The `X-Cache` header is never set + +This is useful for: +- **Debugging** — to rule out caching as a source of stale data +- **Development** — when testing endpoint behavior without cache interference +- **Environments with strict freshness SLA** — where even 10s staleness is unacceptable + +> **Note**: Disabling cache significantly increases database load under high traffic. +> Use with caution in production. + +### docker-compose example + +```yaml +services: + app: + environment: + - CACHE_DISABLED=true # disable in-memory caching +``` + ## Configuration Defined in `internal/api/api.go`: diff --git a/internal/conf/conf.go b/internal/conf/conf.go index aeb211e..bae8770 100644 --- a/internal/conf/conf.go +++ b/internal/conf/conf.go @@ -41,7 +41,10 @@ type Config struct { // Web URL for the app // Example: https://web.example.com WebURL string `envconfig:"WEB_URL"` - // Disable cache logic + // Disable in-memory HTTP response caching. + // When true, GinMiddleware and Invalidator become pass-through (no-op). + // Useful for debugging or environments where data freshness is critical. + // Environment variable: CACHE_DISABLED (default: false) CacheDisabled bool `envconfig:"CACHE_DISABLED"` // Disable authentication for any reasons it doesn't work with hostname like "*prod*" AuthenticationDisabled bool `envconfig:"AUTHENTICATION_DISABLED"`