Commit cf823b1736
Verified · cmc
Layout: unified · split
docs/superpowers/plans/2026-09-10-api-cache.md added +776
| @@ -0,0 +1,776 @@ | ||
| 1 | # API Response Cache Implementation Plan | |
| 2 | ||
| 3 | > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. | |
| 4 | ||
| 5 | **Goal:** Cache DeviantArt API responses in memory so repeat and concurrent | |
| 6 | page views make one upstream request instead of many. | |
| 7 | ||
| 8 | **Architecture:** A caching `http.RoundTripper` sits in front of the | |
| 9 | existing throttle. It stores 200 responses for `/_puppy/` and `/groups/` | |
| 10 | GETs keyed by URL minus `csrf_token`, bounded by bytes with LRU eviction, | |
| 11 | with singleflight coalescing of concurrent misses. | |
| 12 | ||
| 13 | **Tech Stack:** Go 1.25, `golang.org/x/sync/singleflight`, standard | |
| 14 | library `container/list`. | |
| 15 | ||
| 16 | **Spec:** `docs/superpowers/specs/2026-09-10-api-cache-design.md` | |
| 17 | ||
| 18 | ## Global Constraints | |
| 19 | ||
| 20 | - Go module is `skunkyart`; package under test is `app`. | |
| 21 | - Lint must stay clean: `go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.13.2 run ./...`. | |
| 22 | - Tests run with `go test ./app -count=1 -race`. | |
| 23 | - No attribution trailers in commits. | |
| 24 | - Errors and log lines are plain sentences, lowercase error strings. | |
| 25 | ||
| 26 | --- | |
| 27 | ||
| 28 | ### Task 1: Shared lifetime parser and the `api-cache` config block | |
| 29 | ||
| 30 | **Files:** | |
| 31 | - Modify: `app/config.go` | |
| 32 | - Create: `app/config_test.go` | |
| 33 | - Modify: `config.example.json`, `SETUP.md` | |
| 34 | ||
| 35 | **Interfaces:** | |
| 36 | - Produces: `func parseLifetime(s string) (time.Duration, error)`; | |
| 37 | `CFG.APICache` of type `apiCacheConfig{Enabled bool; MaxSize int64; TTL string}`; | |
| 38 | package var `apiCacheTTL time.Duration` set by `ExecuteConfig`. | |
| 39 | ||
| 40 | - [ ] **Step 1: Write the failing tests** | |
| 41 | ||
| 42 | ```go | |
| 43 | package app | |
| 44 | ||
| 45 | import ( | |
| 46 | "testing" | |
| 47 | "time" | |
| 48 | ) | |
| 49 | ||
| 50 | func TestParseLifetimeUnits(t *testing.T) { | |
| 51 | cases := map[string]time.Duration{ | |
| 52 | "5i": 5 * time.Minute, | |
| 53 | "2h": 2 * time.Hour, | |
| 54 | "3d": 72 * time.Hour, | |
| 55 | "1w": 7 * 24 * time.Hour, | |
| 56 | "1m": 30 * 24 * time.Hour, | |
| 57 | "1y": 360 * 24 * time.Hour, | |
| 58 | "12i": 12 * time.Minute, | |
| 59 | } | |
| 60 | for in, want := range cases { | |
| 61 | got, err := parseLifetime(in) | |
| 62 | if err != nil || got != want { | |
| 63 | t.Errorf("parseLifetime(%q) = %v, %v; want %v", in, got, err, want) | |
| 64 | } | |
| 65 | } | |
| 66 | } | |
| 67 | ||
| 68 | func TestParseLifetimeRejectsBadInput(t *testing.T) { | |
| 69 | for _, in := range []string{"", "5", "5x", "h"} { | |
| 70 | if _, err := parseLifetime(in); err == nil { | |
| 71 | t.Errorf("parseLifetime(%q) accepted, want an error", in) | |
| 72 | } | |
| 73 | } | |
| 74 | } | |
| 75 | ||
| 76 | func TestAPICacheDefaults(t *testing.T) { | |
| 77 | if !CFG.APICache.Enabled || CFG.APICache.MaxSize != 64 || CFG.APICache.TTL != "5i" { | |
| 78 | t.Errorf("defaults are %+v, want enabled, 64 MB, 5i", CFG.APICache) | |
| 79 | } | |
| 80 | } | |
| 81 | ``` | |
| 82 | ||
| 83 | - [ ] **Step 2: Run to verify it fails** | |
| 84 | ||
| 85 | Run: `go test ./app -run 'TestParseLifetime|TestAPICacheDefaults' -count=1` | |
| 86 | Expected: FAIL, `undefined: parseLifetime`. | |
| 87 | ||
| 88 | - [ ] **Step 3: Implement** | |
| 89 | ||
| 90 | In `app/config.go`, add `"errors"` to the imports. Add the type and the | |
| 91 | field: | |
| 92 | ||
| 93 | ```go | |
| 94 | type apiCacheConfig struct { | |
| 95 | Enabled bool `json:"enabled"` | |
| 96 | MaxSize int64 `json:"max-size"` | |
| 97 | TTL string `json:"ttl"` | |
| 98 | } | |
| 99 | ``` | |
| 100 | ||
| 101 | In `config`, after `Cache`: | |
| 102 | ||
| 103 | ```go | |
| 104 | APICache apiCacheConfig `json:"api-cache"` | |
| 105 | ``` | |
| 106 | ||
| 107 | In the `CFG` literal, after the `Cache:` block: | |
| 108 | ||
| 109 | ```go | |
| 110 | APICache: apiCacheConfig{ | |
| 111 | Enabled: true, | |
| 112 | MaxSize: 64, | |
| 113 | TTL: "5i", | |
| 114 | }, | |
| 115 | ``` | |
| 116 | ||
| 117 | After `var lifetimeParsed int64`: | |
| 118 | ||
| 119 | ```go | |
| 120 | // apiCacheTTL is api-cache.ttl parsed, set by ExecuteConfig. | |
| 121 | var apiCacheTTL time.Duration | |
| 122 | ||
| 123 | // parseLifetime reads a duration in the config's unit syntax: a number | |
| 124 | // followed by i (minutes), h (hours), d (days), w (weeks), m (30-day | |
| 125 | // months) or y (360-day years). | |
| 126 | func parseLifetime(s string) (time.Duration, error) { | |
| 127 | if s == "" { | |
| 128 | return 0, errors.New("empty lifetime") | |
| 129 | } | |
| 130 | numstr := regexp.MustCompile("[0-9]+").FindAllString(s, -1) | |
| 131 | if len(numstr) == 0 { | |
| 132 | return 0, errors.New("lifetime has no number: " + s) | |
| 133 | } | |
| 134 | num, _ := strconv.Atoi(numstr[len(numstr)-1]) | |
| 135 | ||
| 136 | day := 24 * time.Hour | |
| 137 | var unit time.Duration | |
| 138 | switch s[len(s)-1:] { | |
| 139 | case "i": | |
| 140 | unit = time.Minute | |
| 141 | case "h": | |
| 142 | unit = time.Hour | |
| 143 | case "d": | |
| 144 | unit = day | |
| 145 | case "w": | |
| 146 | unit = 7 * day | |
| 147 | case "m": | |
| 148 | unit = 30 * day | |
| 149 | case "y": | |
| 150 | unit = 360 * day | |
| 151 | default: | |
| 152 | return 0, errors.New("invalid unit specified: " + s[len(s)-1:]) | |
| 153 | } | |
| 154 | return unit * time.Duration(num), nil | |
| 155 | } | |
| 156 | ``` | |
| 157 | ||
| 158 | Replace the `if CFG.Cache.Lifetime != "" { ... }` block inside | |
| 159 | `ExecuteConfig` with: | |
| 160 | ||
| 161 | ```go | |
| 162 | if CFG.Cache.Lifetime != "" { | |
| 163 | d, err := parseLifetime(CFG.Cache.Lifetime) | |
| 164 | if err != nil { | |
| 165 | exit("config: cache.lifetime: "+err.Error(), 1) | |
| 166 | } | |
| 167 | lifetimeParsed = d.Milliseconds() | |
| 168 | } | |
| 169 | ``` | |
| 170 | ||
| 171 | After the theme switch, before `static.StaticPath = CFG.StaticPath`: | |
| 172 | ||
| 173 | ```go | |
| 174 | if CFG.APICache.Enabled { | |
| 175 | d, err := parseLifetime(CFG.APICache.TTL) | |
| 176 | if err != nil { | |
| 177 | exit("config: api-cache.ttl: "+err.Error(), 1) | |
| 178 | } | |
| 179 | apiCacheTTL = d | |
| 180 | } | |
| 181 | ``` | |
| 182 | ||
| 183 | Remove the now-unused `day` and `duration` locals from `ExecuteConfig`. | |
| 184 | ||
| 185 | In `config.example.json`, after the `cache` block: | |
| 186 | ||
| 187 | ```json | |
| 188 | "api-cache": { | |
| 189 | "enabled": true, | |
| 190 | "max-size": 64, | |
| 191 | "ttl": "5i" | |
| 192 | }, | |
| 193 | ``` | |
| 194 | ||
| 195 | In `SETUP.md`, after the `cache` bullet list, add: | |
| 196 | ||
| 197 | ```markdown | |
| 198 | * `api-cache` — In-memory cache of DeviantArt API responses. Every page, | |
| 199 | feed poll and API call that asks DeviantArt the same question within the | |
| 200 | TTL is answered from memory, and concurrent requests for one thing make | |
| 201 | one upstream call. On by default; DeviantArt bans egress IPs that ask too | |
| 202 | often, so leave it on unless you are debugging. | |
| 203 | * `enabled` — boolean, default true | |
| 204 | * `max-size` — megabytes of response bodies to hold, default 64. Least | |
| 205 | recently used entries are dropped past this. | |
| 206 | * `ttl` — how long a response is reused, in the time units above. Default | |
| 207 | `5i`. | |
| 208 | ``` | |
| 209 | ||
| 210 | Also add `d` — days to the Time units list at the top of `SETUP.md`. | |
| 211 | ||
| 212 | - [ ] **Step 4: Run tests** | |
| 213 | ||
| 214 | Run: `go test ./app -count=1 -race` | |
| 215 | Expected: PASS. | |
| 216 | ||
| 217 | - [ ] **Step 5: Commit** | |
| 218 | ||
| 219 | ```bash | |
| 220 | git add app/config.go app/config_test.go config.example.json SETUP.md | |
| 221 | git commit -m "Add the api-cache config block and a shared lifetime parser | |
| 222 | ||
| 223 | Ref #8" | |
| 224 | ``` | |
| 225 | ||
| 226 | --- | |
| 227 | ||
| 228 | ### Task 2: The cache store and transport | |
| 229 | ||
| 230 | **Files:** | |
| 231 | - Create: `app/apicache.go`, `app/apicache_test.go` | |
| 232 | - Modify: `go.mod`, `go.sum` | |
| 233 | ||
| 234 | **Interfaces:** | |
| 235 | - Consumes: nothing from Task 1 (the store takes its bound and TTL as arguments). | |
| 236 | - Produces: `func newAPICache(maxBytes int64, ttl time.Duration) *apiCache`; | |
| 237 | `func (c *apiCache) transport(base http.RoundTripper) http.RoundTripper`; | |
| 238 | `func (c *apiCache) stats() (hits, misses int64, entries int, held int64)`; | |
| 239 | `func cacheable(req *http.Request) bool`; `func cacheKey(req *http.Request) string`. | |
| 240 | ||
| 241 | - [ ] **Step 1: Add the dependency** | |
| 242 | ||
| 243 | Run: `go get golang.org/x/sync@latest && go mod tidy` | |
| 244 | ||
| 245 | - [ ] **Step 2: Write the failing tests** | |
| 246 | ||
| 247 | ```go | |
| 248 | package app | |
| 249 | ||
| 250 | import ( | |
| 251 | "io" | |
| 252 | "net/http" | |
| 253 | "strings" | |
| 254 | "sync" | |
| 255 | "testing" | |
| 256 | "time" | |
| 257 | ) | |
| 258 | ||
| 259 | // fakeRT is the upstream: it counts calls and returns a scripted response. | |
| 260 | type fakeRT struct { | |
| 261 | mu sync.Mutex | |
| 262 | calls int | |
| 263 | status int | |
| 264 | body string | |
| 265 | delay time.Duration | |
| 266 | } | |
| 267 | ||
| 268 | func (f *fakeRT) RoundTrip(r *http.Request) (*http.Response, error) { | |
| 269 | f.mu.Lock() | |
| 270 | f.calls++ | |
| 271 | f.mu.Unlock() | |
| 272 | time.Sleep(f.delay) | |
| 273 | return &http.Response{ | |
| 274 | StatusCode: f.status, | |
| 275 | Header: http.Header{"Content-Type": {"application/json"}}, | |
| 276 | Body: io.NopCloser(strings.NewReader(f.body)), | |
| 277 | Request: r, | |
| 278 | }, nil | |
| 279 | } | |
| 280 | ||
| 281 | func (f *fakeRT) count() int { | |
| 282 | f.mu.Lock() | |
| 283 | defer f.mu.Unlock() | |
| 284 | return f.calls | |
| 285 | } | |
| 286 | ||
| 287 | const puppyURL = "https://www.deviantart.com/_puppy/dabrowse/search/all?q=fox&csrf_token=abc" | |
| 288 | ||
| 289 | func get(t *testing.T, rt http.RoundTripper, url string) (int, string) { | |
| 290 | t.Helper() | |
| 291 | req, err := http.NewRequest(http.MethodGet, url, nil) //nolint:noctx // test request | |
| 292 | if err != nil { | |
| 293 | t.Fatal(err) | |
| 294 | } | |
| 295 | resp, err := rt.RoundTrip(req) | |
| 296 | if err != nil { | |
| 297 | t.Fatal(err) | |
| 298 | } | |
| 299 | defer func() { _ = resp.Body.Close() }() | |
| 300 | body, _ := io.ReadAll(resp.Body) | |
| 301 | return resp.StatusCode, string(body) | |
| 302 | } | |
| 303 | ||
| 304 | func TestSecondRequestIsServedFromCache(t *testing.T) { | |
| 305 | up := &fakeRT{status: 200, body: `{"a":1}`} | |
| 306 | rt := newAPICache(1<<20, time.Minute).transport(up) | |
| 307 | ||
| 308 | get(t, rt, puppyURL) | |
| 309 | status, body := get(t, rt, puppyURL) | |
| 310 | ||
| 311 | if up.count() != 1 { | |
| 312 | t.Errorf("upstream called %d times, want 1", up.count()) | |
| 313 | } | |
| 314 | if status != 200 || body != `{"a":1}` { | |
| 315 | t.Errorf("cached response is %d %q", status, body) | |
| 316 | } | |
| 317 | } | |
| 318 | ||
| 319 | func TestExpiredEntryIsRefetched(t *testing.T) { | |
| 320 | up := &fakeRT{status: 200, body: `{}`} | |
| 321 | c := newAPICache(1<<20, time.Minute) | |
| 322 | now := time.Now() | |
| 323 | c.now = func() time.Time { return now } | |
| 324 | rt := c.transport(up) | |
| 325 | ||
| 326 | get(t, rt, puppyURL) | |
| 327 | now = now.Add(2 * time.Minute) | |
| 328 | get(t, rt, puppyURL) | |
| 329 | ||
| 330 | if up.count() != 2 { | |
| 331 | t.Errorf("upstream called %d times, want 2 after expiry", up.count()) | |
| 332 | } | |
| 333 | } | |
| 334 | ||
| 335 | func TestNon200IsNotStored(t *testing.T) { | |
| 336 | up := &fakeRT{status: 403, body: "blocked"} | |
| 337 | rt := newAPICache(1<<20, time.Minute).transport(up) | |
| 338 | ||
| 339 | status, body := get(t, rt, puppyURL) | |
| 340 | get(t, rt, puppyURL) | |
| 341 | ||
| 342 | if status != 403 || body != "blocked" { | |
| 343 | t.Errorf("first response is %d %q, want the upstream 403 passed through", status, body) | |
| 344 | } | |
| 345 | if up.count() != 2 { | |
| 346 | t.Errorf("upstream called %d times, want 2: a 403 must not be cached", up.count()) | |
| 347 | } | |
| 348 | } | |
| 349 | ||
| 350 | func TestBypassesSessionAndOtherHosts(t *testing.T) { | |
| 351 | up := &fakeRT{status: 200, body: "x"} | |
| 352 | rt := newAPICache(1<<20, time.Minute).transport(up) | |
| 353 | ||
| 354 | for _, url := range []string{ | |
| 355 | "https://www.deviantart.com/_puppy", | |
| 356 | "https://www.deviantart.com", | |
| 357 | "https://a.deviantart.net/avatars-big/a/alice.png", | |
| 358 | } { | |
| 359 | get(t, rt, url) | |
| 360 | get(t, rt, url) | |
| 361 | } | |
| 362 | if up.count() != 6 { | |
| 363 | t.Errorf("upstream called %d times, want 6: none of these URLs may be cached", up.count()) | |
| 364 | } | |
| 365 | } | |
| 366 | ||
| 367 | func TestKeyIgnoresCSRFToken(t *testing.T) { | |
| 368 | up := &fakeRT{status: 200, body: "x"} | |
| 369 | rt := newAPICache(1<<20, time.Minute).transport(up) | |
| 370 | ||
| 371 | get(t, rt, puppyURL) | |
| 372 | get(t, rt, strings.Replace(puppyURL, "csrf_token=abc", "csrf_token=def", 1)) | |
| 373 | ||
| 374 | if up.count() != 1 { | |
| 375 | t.Errorf("upstream called %d times, want 1: a token refresh must not miss", up.count()) | |
| 376 | } | |
| 377 | } | |
| 378 | ||
| 379 | func TestByteBoundEvictsLeastRecentlyUsed(t *testing.T) { | |
| 380 | up := &fakeRT{status: 200, body: strings.Repeat("x", 100)} | |
| 381 | rt := newAPICache(250, time.Minute).transport(up) | |
| 382 | a := "https://www.deviantart.com/_puppy/a?p=1" | |
| 383 | b := "https://www.deviantart.com/_puppy/b?p=1" | |
| 384 | c := "https://www.deviantart.com/_puppy/c?p=1" | |
| 385 | ||
| 386 | get(t, rt, a) | |
| 387 | get(t, rt, b) | |
| 388 | get(t, rt, a) // a is now more recent than b | |
| 389 | get(t, rt, c) // 300 bytes would exceed 250: b goes | |
| 390 | get(t, rt, a) | |
| 391 | get(t, rt, c) | |
| 392 | get(t, rt, b) | |
| 393 | ||
| 394 | if up.count() != 4 { | |
| 395 | t.Errorf("upstream called %d times, want 4: only b should have been evicted", up.count()) | |
| 396 | } | |
| 397 | } | |
| 398 | ||
| 399 | func TestConcurrentMissesMakeOneUpstreamCall(t *testing.T) { | |
| 400 | up := &fakeRT{status: 200, body: "x", delay: 50 * time.Millisecond} | |
| 401 | rt := newAPICache(1<<20, time.Minute).transport(up) | |
| 402 | ||
| 403 | var wg sync.WaitGroup | |
| 404 | for range 20 { | |
| 405 | wg.Go(func() { get(t, rt, puppyURL) }) | |
| 406 | } | |
| 407 | wg.Wait() | |
| 408 | ||
| 409 | if up.count() != 1 { | |
| 410 | t.Errorf("upstream called %d times, want 1 for a burst on one key", up.count()) | |
| 411 | } | |
| 412 | } | |
| 413 | ||
| 414 | func TestStatsCountHitsAndMisses(t *testing.T) { | |
| 415 | up := &fakeRT{status: 200, body: "abc"} | |
| 416 | c := newAPICache(1<<20, time.Minute) | |
| 417 | rt := c.transport(up) | |
| 418 | ||
| 419 | get(t, rt, puppyURL) | |
| 420 | get(t, rt, puppyURL) | |
| 421 | get(t, rt, puppyURL) | |
| 422 | ||
| 423 | hits, misses, entries, held := c.stats() | |
| 424 | if hits != 2 || misses != 1 || entries != 1 || held != 3 { | |
| 425 | t.Errorf("stats = %d hits, %d misses, %d entries, %d bytes; want 2, 1, 1, 3", hits, misses, entries, held) | |
| 426 | } | |
| 427 | } | |
| 428 | ``` | |
| 429 | ||
| 430 | Note: `wg.Go` needs Go 1.25, which `go.mod` already requires. | |
| 431 | ||
| 432 | - [ ] **Step 3: Run to verify it fails** | |
| 433 | ||
| 434 | Run: `go test ./app -run 'Cache|Bypasses|Key|Bound|Concurrent|Stats' -count=1` | |
| 435 | Expected: FAIL, `undefined: newAPICache`. | |
| 436 | ||
| 437 | - [ ] **Step 4: Implement `app/apicache.go`** | |
| 438 | ||
| 439 | ```go | |
| 440 | package app | |
| 441 | ||
| 442 | import ( | |
| 443 | "bytes" | |
| 444 | "container/list" | |
| 445 | "io" | |
| 446 | "net/http" | |
| 447 | "strings" | |
| 448 | "sync" | |
| 449 | "time" | |
| 450 | ||
| 451 | "golang.org/x/sync/singleflight" | |
| 452 | ) | |
| 453 | ||
| 454 | // apiCache holds DeviantArt API responses so that repeat and concurrent | |
| 455 | // requests for one URL cost one upstream call. It sits in front of the | |
| 456 | // throttle: a hit never touches DeviantArt or the throttle's budget. | |
| 457 | // | |
| 458 | // The store is bounded by body bytes with least-recently-used eviction. It | |
| 459 | // is safe for concurrent use. | |
| 460 | type apiCache struct { | |
| 461 | maxBytes int64 | |
| 462 | ttl time.Duration | |
| 463 | now func() time.Time | |
| 464 | ||
| 465 | mu sync.Mutex | |
| 466 | entries map[string]*cacheEntry | |
| 467 | lru *list.List // front is most recently used | |
| 468 | held int64 | |
| 469 | hits int64 | |
| 470 | misses int64 | |
| 471 | ||
| 472 | flight singleflight.Group | |
| 473 | } | |
| 474 | ||
| 475 | // cacheEntry is one buffered response. header is a clone of the upstream | |
| 476 | // header; body is the whole body, read once. | |
| 477 | type cacheEntry struct { | |
| 478 | key string | |
| 479 | status int | |
| 480 | header http.Header | |
| 481 | body []byte | |
| 482 | expires time.Time | |
| 483 | elem *list.Element | |
| 484 | } | |
| 485 | ||
| 486 | func newAPICache(maxBytes int64, ttl time.Duration) *apiCache { | |
| 487 | return &apiCache{ | |
| 488 | maxBytes: maxBytes, | |
| 489 | ttl: ttl, | |
| 490 | now: time.Now, | |
| 491 | entries: map[string]*cacheEntry{}, | |
| 492 | lru: list.New(), | |
| 493 | } | |
| 494 | } | |
| 495 | ||
| 496 | // cacheable reports whether a request is one the cache handles: a GET to | |
| 497 | // DeviantArt's API or its group search page. The session bootstrap (/_puppy | |
| 498 | // with no path), the homepage, avatars and media all pass through. | |
| 499 | func cacheable(req *http.Request) bool { | |
| 500 | if req.Method != http.MethodGet || req.URL.Host != "www.deviantart.com" { | |
| 501 | return false | |
| 502 | } | |
| 503 | p := req.URL.Path | |
| 504 | return (strings.HasPrefix(p, "/_puppy/") && len(p) > len("/_puppy/")) || | |
| 505 | strings.HasPrefix(p, "/groups/") | |
| 506 | } | |
| 507 | ||
| 508 | // cacheKey is the URL without csrf_token, which changes every twelve hours | |
| 509 | // and would otherwise empty the cache on each refresh. | |
| 510 | func cacheKey(req *http.Request) string { | |
| 511 | u := *req.URL | |
| 512 | q := u.Query() | |
| 513 | q.Del("csrf_token") | |
| 514 | u.RawQuery = q.Encode() | |
| 515 | return u.String() | |
| 516 | } | |
| 517 | ||
| 518 | // transport returns a RoundTripper that answers from this cache and sends | |
| 519 | // misses to base. Several transports may share one cache. | |
| 520 | func (c *apiCache) transport(base http.RoundTripper) http.RoundTripper { | |
| 521 | return &cachedTransport{cache: c, base: base} | |
| 522 | } | |
| 523 | ||
| 524 | type cachedTransport struct { | |
| 525 | cache *apiCache | |
| 526 | base http.RoundTripper | |
| 527 | } | |
| 528 | ||
| 529 | // RoundTrip serves a hit from memory. A miss is fetched once per key however | |
| 530 | // many callers are waiting, buffered, stored if it is a 200, and handed to | |
| 531 | // every waiter as its own response. | |
| 532 | func (t *cachedTransport) RoundTrip(req *http.Request) (*http.Response, error) { | |
| 533 | if !cacheable(req) { | |
| 534 | return t.base.RoundTrip(req) | |
| 535 | } | |
| 536 | key := cacheKey(req) | |
| 537 | if e := t.cache.get(key); e != nil { | |
| 538 | return e.response(req), nil | |
| 539 | } | |
| 540 | ||
| 541 | v, err, _ := t.cache.flight.Do(key, func() (any, error) { | |
| 542 | resp, err := t.base.RoundTrip(req) | |
| 543 | if err != nil { | |
| 544 | return nil, err | |
| 545 | } | |
| 546 | defer func() { _ = resp.Body.Close() }() | |
| 547 | body, err := io.ReadAll(resp.Body) | |
| 548 | if err != nil { | |
| 549 | return nil, err | |
| 550 | } | |
| 551 | e := &cacheEntry{key: key, status: resp.StatusCode, header: resp.Header.Clone(), body: body} | |
| 552 | if e.status == http.StatusOK { | |
| 553 | t.cache.put(e) | |
| 554 | } | |
| 555 | return e, nil | |
| 556 | }) | |
| 557 | if err != nil { | |
| 558 | return nil, err | |
| 559 | } | |
| 560 | e, ok := v.(*cacheEntry) | |
| 561 | if !ok { | |
| 562 | return nil, io.ErrUnexpectedEOF | |
| 563 | } | |
| 564 | return e.response(req), nil | |
| 565 | } | |
| 566 | ||
| 567 | // response builds a fresh http.Response over the buffered body, so each | |
| 568 | // caller can read and close its own. | |
| 569 | func (e *cacheEntry) response(req *http.Request) *http.Response { | |
| 570 | return &http.Response{ | |
| 571 | Status: http.StatusText(e.status), | |
| 572 | StatusCode: e.status, | |
| 573 | Proto: "HTTP/1.1", | |
| 574 | ProtoMajor: 1, | |
| 575 | ProtoMinor: 1, | |
| 576 | Header: e.header.Clone(), | |
| 577 | Body: io.NopCloser(bytes.NewReader(e.body)), | |
| 578 | ContentLength: int64(len(e.body)), | |
| 579 | Request: req, | |
| 580 | } | |
| 581 | } | |
| 582 | ||
| 583 | // get returns the live entry for key, marking it most recently used, or nil. | |
| 584 | // An expired entry is dropped on the way out. | |
| 585 | func (c *apiCache) get(key string) *cacheEntry { | |
| 586 | c.mu.Lock() | |
| 587 | defer c.mu.Unlock() | |
| 588 | ||
| 589 | e := c.entries[key] | |
| 590 | if e == nil { | |
| 591 | c.misses++ | |
| 592 | return nil | |
| 593 | } | |
| 594 | if !c.now().Before(e.expires) { | |
| 595 | c.remove(e) | |
| 596 | c.misses++ | |
| 597 | return nil | |
| 598 | } | |
| 599 | c.lru.MoveToFront(e.elem) | |
| 600 | c.hits++ | |
| 601 | return e | |
| 602 | } | |
| 603 | ||
| 604 | // put stores e, evicting from the least recently used end until it fits. A | |
| 605 | // body larger than the whole bound is not stored. | |
| 606 | func (c *apiCache) put(e *cacheEntry) { | |
| 607 | size := int64(len(e.body)) | |
| 608 | if size > c.maxBytes { | |
| 609 | return | |
| 610 | } | |
| 611 | ||
| 612 | c.mu.Lock() | |
| 613 | defer c.mu.Unlock() | |
| 614 | ||
| 615 | if old := c.entries[e.key]; old != nil { | |
| 616 | c.remove(old) | |
| 617 | } | |
| 618 | for c.held+size > c.maxBytes { | |
| 619 | back, ok := c.lru.Back().Value.(*cacheEntry) | |
| 620 | if !ok { | |
| 621 | break | |
| 622 | } | |
| 623 | c.remove(back) | |
| 624 | } | |
| 625 | e.expires = c.now().Add(c.ttl) | |
| 626 | e.elem = c.lru.PushFront(e) | |
| 627 | c.entries[e.key] = e | |
| 628 | c.held += size | |
| 629 | } | |
| 630 | ||
| 631 | // remove drops e. The caller holds mu. | |
| 632 | func (c *apiCache) remove(e *cacheEntry) { | |
| 633 | c.lru.Remove(e.elem) | |
| 634 | delete(c.entries, e.key) | |
| 635 | c.held -= int64(len(e.body)) | |
| 636 | } | |
| 637 | ||
| 638 | // stats reports the counters for the hourly log line. | |
| 639 | func (c *apiCache) stats() (hits, misses int64, entries int, held int64) { | |
| 640 | c.mu.Lock() | |
| 641 | defer c.mu.Unlock() | |
| 642 | return c.hits, c.misses, len(c.entries), c.held | |
| 643 | } | |
| 644 | ``` | |
| 645 | ||
| 646 | - [ ] **Step 5: Run tests** | |
| 647 | ||
| 648 | Run: `go test ./app -count=1 -race` | |
| 649 | Expected: PASS. | |
| 650 | ||
| 651 | - [ ] **Step 6: Lint** | |
| 652 | ||
| 653 | Run: `go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.13.2 run ./...` | |
| 654 | Expected: `0 issues.` | |
| 655 | ||
| 656 | - [ ] **Step 7: Commit** | |
| 657 | ||
| 658 | ```bash | |
| 659 | git add app/apicache.go app/apicache_test.go go.mod go.sum | |
| 660 | git commit -m "Add an in-memory cache for DeviantArt API responses | |
| 661 | ||
| 662 | Ref #8" | |
| 663 | ``` | |
| 664 | ||
| 665 | --- | |
| 666 | ||
| 667 | ### Task 3: Install the cache in the transport chain | |
| 668 | ||
| 669 | **Files:** | |
| 670 | - Modify: `app/httpclient.go` | |
| 671 | - Modify: `app/apicache_test.go` (one more test) | |
| 672 | - Modify: `app/httpclient_test.go` if it constructs the chain directly (read it first) | |
| 673 | ||
| 674 | **Interfaces:** | |
| 675 | - Consumes: `newAPICache`, `(*apiCache).transport`, `(*apiCache).stats`, | |
| 676 | `CFG.APICache`, `apiCacheTTL`, `throttled`, `daThrottle`. | |
| 677 | - Produces: package var `daCache *apiCache` (nil when disabled); | |
| 678 | `func chain(base http.RoundTripper) http.RoundTripper`. | |
| 679 | ||
| 680 | - [ ] **Step 1: Write the failing test** (append to `app/apicache_test.go`) | |
| 681 | ||
| 682 | ```go | |
| 683 | // TestHitDoesNotConsumeAThrottleSlot pins the chain order: the cache sits in | |
| 684 | // front of the throttle, so a hit returns even when every throttle slot is | |
| 685 | // held. With the order reversed this test hangs and times out. | |
| 686 | func TestHitDoesNotConsumeAThrottleSlot(t *testing.T) { | |
| 687 | up := &fakeRT{status: 200, body: "x"} | |
| 688 | th := &daThrottle{base: up, sem: make(chan struct{}, 1)} | |
| 689 | rt := newAPICache(1<<20, time.Minute).transport(th) | |
| 690 | ||
| 691 | get(t, rt, puppyURL) // populate through the throttle | |
| 692 | ||
| 693 | th.sem <- struct{}{} // hold the only slot | |
| 694 | done := make(chan struct{}) | |
| 695 | go func() { | |
| 696 | get(t, rt, puppyURL) | |
| 697 | close(done) | |
| 698 | }() | |
| 699 | select { | |
| 700 | case <-done: | |
| 701 | case <-time.After(2 * time.Second): | |
| 702 | t.Fatal("a cache hit waited on the throttle") | |
| 703 | } | |
| 704 | } | |
| 705 | ``` | |
| 706 | ||
| 707 | - [ ] **Step 2: Run to verify it fails** | |
| 708 | ||
| 709 | Run: `go test ./app -run TestHitDoesNotConsumeAThrottleSlot -count=1` | |
| 710 | Expected: PASS already, since the test builds the chain by hand. That is | |
| 711 | fine: it documents the required order, and Step 3 makes the production | |
| 712 | chain match it. | |
| 713 | ||
| 714 | - [ ] **Step 3: Implement** | |
| 715 | ||
| 716 | In `app/httpclient.go`, after `var baseTransport *http.Transport`: | |
| 717 | ||
| 718 | ```go | |
| 719 | // daCache is the API response cache shared by every transport, or nil when | |
| 720 | // api-cache.enabled is false. | |
| 721 | var daCache *apiCache | |
| 722 | ||
| 723 | // chain wraps base with the throttle and, when enabled, the cache in front | |
| 724 | // of it, so a hit never spends a throttle slot. | |
| 725 | func chain(base http.RoundTripper) http.RoundTripper { | |
| 726 | rt := throttled(base) | |
| 727 | if daCache != nil { | |
| 728 | return daCache.transport(rt) | |
| 729 | } | |
| 730 | return rt | |
| 731 | } | |
| 732 | ||
| 733 | // logCacheStatsForever prints one line an hour so an operator can see the | |
| 734 | // cache working without an endpoint. Run it in its own goroutine. | |
| 735 | func logCacheStatsForever(c *apiCache) { | |
| 736 | for { | |
| 737 | time.Sleep(time.Hour) | |
| 738 | hits, misses, entries, held := c.stats() | |
| 739 | println("api cache:", hits, "hits,", misses, "misses,", entries, "entries,", held>>20, "MB held") | |
| 740 | } | |
| 741 | } | |
| 742 | ``` | |
| 743 | ||
| 744 | Change `InstallDAThrottle` to: | |
| 745 | ||
| 746 | ```go | |
| 747 | func InstallDAThrottle() { | |
| 748 | baseTransport = tunedTransport() | |
| 749 | if CFG.APICache.Enabled { | |
| 750 | daCache = newAPICache(CFG.APICache.MaxSize<<20, apiCacheTTL) | |
| 751 | go logCacheStatsForever(daCache) | |
| 752 | } | |
| 753 | http.DefaultTransport = chain(baseTransport) | |
| 754 | } | |
| 755 | ``` | |
| 756 | ||
| 757 | Update its doc comment to mention the cache. In `ProxiedTransport`, | |
| 758 | replace `return throttled(base)` with `return chain(base)`. | |
| 759 | ||
| 760 | - [ ] **Step 4: Run tests and lint** | |
| 761 | ||
| 762 | Run: `go test ./... -count=1 -race && go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.13.2 run ./...` | |
| 763 | Expected: PASS, `0 issues.` | |
| 764 | ||
| 765 | - [ ] **Step 5: Commit** | |
| 766 | ||
| 767 | ```bash | |
| 768 | git add app/httpclient.go app/apicache_test.go | |
| 769 | git commit -m "Serve DeviantArt API responses from the cache | |
| 770 | ||
| 771 | The cache sits in front of the throttle, so a hit costs neither an | |
| 772 | upstream request nor a throttle slot. On by default; api-cache.enabled | |
| 773 | turns it off. | |
| 774 | ||
| 775 | Closes #8" | |
| 776 | ``` | |