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 new file mode 100644 index 0000000..9784de9 --- /dev/null +++ b/docs/caching.md @@ -0,0 +1,255 @@ +# 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 | + +## 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`: + +```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 + +## 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 + +## 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/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) diff --git a/internal/api/api.go b/internal/api/api.go index 1fd2afc..3ebf1a3 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.HTTPCache + eventsCache *cache.HTTPCache } func New(cfg *conf.Config, log *zap.Logger, database *db.DB) (*API, error) { @@ -44,11 +53,30 @@ 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} + 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, + secretKeyV1: cfg.SecretKeyV1, authGroup: cfg.AuthGroup, + componentsCache: componentsCache, eventsCache: eventsCache, + } a.InitRoutes() return a, nil } +func (a *API) Close() { + if a.componentsCache != nil { + a.componentsCache.Close() + } + if a.eventsCache != nil { + a.eventsCache.Close() + } +} + func (a *API) Router() *gin.Engine { return a.r } 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..aa12d9c --- /dev/null +++ b/internal/cache/cache.go @@ -0,0 +1,116 @@ +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]*list.Element + order *list.List + ttl time.Duration + maxItems int + done chan struct{} +} + +func New[V any](ttl time.Duration, maxItems int) *Cache[V] { + c := &Cache[V]{ + items: make(map[string]*list.Element), + order: list.New(), + ttl: ttl, + maxItems: maxItems, + 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 el := c.order.Front(); el != nil; { + e, _ := el.Value.(*entry[V]) + if now.After(e.expiresAt) { + next := el.Next() + c.order.Remove(el) + delete(c.items, e.key) + el = next + } else { + break + } + } + c.mu.Unlock() + case <-c.done: + return + } + } +} + +func (c *Cache[V]) Get(key string) (V, bool) { + c.mu.RLock() + el, ok := c.items[key] + c.mu.RUnlock() + + if !ok { + var zero V + return zero, false + } + + e, _ := el.Value.(*entry[V]) + 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() + 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) + } + } + 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() + 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]*list.Element) + c.order.Init() + 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..df1dfa2 --- /dev/null +++ b/internal/cache/middleware.go @@ -0,0 +1,171 @@ +package cache + +import ( + "bytes" + "net/http" + "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 +} + +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 h == nil { + ctx.Next() + return + } + if ctx.Request.Method != http.MethodGet { + ctx.Next() + return + } + + key := ctx.Request.RequestURI + + if cached, ok := h.c.Get(key); ok { + writeCached(ctx, cached, true) + ctx.Abort() + return + } + + // 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) { + if h == nil { + ctx.Next() + return + } + + ctx.Next() + + if ctx.Request.Method != http.MethodGet && ctx.Writer.Status() < 400 { + h.c.InvalidateAll() + } + } +} + +// 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) +} + +// 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 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(), + } +} + +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 + r.written = true +} + +func (r *drainRecorder) Write(b []byte) (int, error) { + r.written = true + return r.body.Write(b) +} + +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 new file mode 100644 index 0000000..0297599 --- /dev/null +++ b/internal/cache/middleware_test.go @@ -0,0 +1,409 @@ +package cache + +import ( + "fmt" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCache(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + run func(t *testing.T) + }{ + { + name: "operations", + run: func(t *testing.T) { + c := New[string](time.Minute, 100) + 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) + assert.Equal(t, 0, c.order.Len()) + + httpCache := NewHTTPCache(time.Second) + defer httpCache.Close() + require.NotNil(t, httpCache) + assert.Equal(t, time.Second, httpCache.c.ttl) + }, + }, + { + name: "get returns false on expired entry", + run: func(t *testing.T) { + c := New[string](time.Minute, 100) + defer c.Close() + + c.mu.Lock() + 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") + 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 := range workers { + go func(id int) { + defer wg.Done() + for i := range opsPerWk { + 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() + 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() + }, + }, + { + 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.Len(t, c.items, maxItems) + assert.Equal(t, maxItems, c.order.Len()) + c.mu.RUnlock() + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + tc.run(t) + }) + } +} + +func TestGinMiddleware(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + tests := []struct { + name string + method string + path string + setupRouter func(r *gin.Engine, cached *HTTPCache) + requests int + expectedCode int + expectedBody string + expectedCache []string + }{ + { + name: "caches successful GET response", + method: http.MethodGet, + path: "/cacheable", + setupRouter: func(r *gin.Engine, cached *HTTPCache) { + 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 *HTTPCache) { + 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 *HTTPCache) { + 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{"", ""}, + }, + { + name: "skips caching when cache is disabled (h is nil)", + method: http.MethodGet, + path: "/disabled", + setupRouter: func(r *gin.Engine, _ *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 { + 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 := range tc.requests { + 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) { + t.Parallel() + gin.SetMode(gin.TestMode) + + tests := []struct { + name string + 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 *HTTPCache) { + 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 *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(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()) + }, + }, + { + 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 { + 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 +} + +// --------------------------------------------------------------------------- +// Benchmarks +// --------------------------------------------------------------------------- + +func BenchmarkCacheSetGet(b *testing.B) { + const keyCount = 4096 + + keys := make([]string, keyCount) + for i := range keys { + keys[i] = fmt.Sprintf("key-%d", i) + } + + c := New[string](time.Minute, keyCount) + defer c.Close() + + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + i := 0 + for pb.Next() { + k := keys[i%keyCount] + c.Set(k, "value") + c.Get(k) + 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") + }) + + // 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, _ := http.NewRequest(http.MethodGet, "/bench", nil) + router.ServeHTTP(w, r) + } + }) +} diff --git a/internal/conf/conf.go b/internal/conf/conf.go index 7c2fe5f..bae8770 100644 --- a/internal/conf/conf.go +++ b/internal/conf/conf.go @@ -41,6 +41,11 @@ type Config struct { // Web URL for the app // Example: https://web.example.com WebURL string `envconfig:"WEB_URL"` + // 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"` // Secret key for V1 authentication (deprecated) @@ -210,7 +215,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), ) diff --git a/tests/cache_test.go b/tests/cache_test.go new file mode 100644 index 0000000..fbf3f22 --- /dev/null +++ b/tests/cache_test.go @@ -0,0 +1,163 @@ +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) (*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) + + 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) { + t.Cleanup(func() { resetIncidentSeed(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: `{"name":"Distributed Cache Service","impact":1,"text":"Cache invalidation test","attributes":[{"name":"region","value":"EU-NL"}]}`, + 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.Less(t, postW.Code, 400, "mutation request must succeed") + + 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 +} diff --git a/tests/main_test.go b/tests/main_test.go index 930bbba..72ad756 100644 --- a/tests/main_test.go +++ b/tests/main_test.go @@ -207,18 +207,42 @@ 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) { +// 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 +// modify incidents. Typically called via t.Cleanup: +// +// t.Cleanup(func() { resetIncidentSeed(t) }) +func resetIncidentSeed(t *testing.T) { t.Helper() - t.Log("cleaning up incident-related tables before test") + t.Log("resetting incident tables to seed state") + + 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')`, + `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)`, + ) +} - gormDB, err := gorm.Open(gormpostgres.Open(databaseURL), &gorm.Config{}) - require.NoError(t, err, "failed to open gorm connection for truncation") +// 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() - result := gormDB.Exec("TRUNCATE TABLE incident, incident_status, incident_component_relation RESTART IDENTITY") - require.NoError(t, result.Error, "failed to truncate incident tables") + 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") + }() - 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") + for _, q := range queries { + 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 7894c1d..5e41d53 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, }, @@ -1075,8 +1080,9 @@ 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} 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..925187a 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,133 +768,141 @@ 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 { 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, 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)) @@ -1074,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) @@ -1183,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