Skip to content

test: add installation-token caching and refresh coverage for interna… - #434

Open
JClark011 wants to merge 1 commit into
Grainlify:mainfrom
JClark011:test/github-app-token-cache-coverage
Open

test: add installation-token caching and refresh coverage for interna…#434
JClark011 wants to merge 1 commit into
Grainlify:mainfrom
JClark011:test/github-app-token-cache-coverage

Conversation

@JClark011

Copy link
Copy Markdown

Summary

internal/github/app.go previously fetched a new installation access token from
GitHub on every call to GetInstallationToken, with no caching. This meant
short-lived tokens (~1h) were never reused, and a burst of concurrent callers
could trigger N redundant token-exchange requests — potentially tripping
GitHub's rate limits and causing cascading 401s.

This PR adds an in-memory cache with a proactive refresh-ahead window and
singleflight-based stampede prevention, and adds comprehensive tests to cover
all cache behaviours.

──────────────────────────────────────────────────────────────────────────────

Changes

internal/github/app.go

  • tokenRefreshAheadWindow = 5 * time.Minute — tokens are treated as stale when
    their remaining TTL falls below this threshold, giving callers a buffer
    against clock-skew and in-flight request latency before the token actually
    expires

  • cachedToken struct — holds token + expiresAt; isValid(now) encapsulates the
    refresh-ahead boundary check

  • GetInstallationToken reworked with two paths:

    • Fast path: mutex-guarded cache lookup; returns immediately on a valid hit
      with zero HTTP calls
    • Slow path: singleflight.Do keyed by installation ID — all concurrent
      callers that observe a stale cache coalesce into exactly one GitHub API call;
      all waiters receive the same fresh token
  • Double-checked locking inside the singleflight callback handles the case
    where a waiting goroutine finds the cache already populated by the leader

    • On API failure, the error is returned directly — no stale/expired token is
      silently reused
  • fetchInstallationToken extracted as a helper returning (token, expiresAt,
    error) so the cache can store the expiry

internal/github/app_test.go

13 new tests, all passing under -race:

┌────────────────────────────┬────────────────────────────────────────────┐
│ Test │ Scenario │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestCachedToken_IsValid (6 │ isValid boundary at window+1s, exact │
│ sub-tests) │ boundary, window-1s, 30s remaining, │
│ │ expired, empty token │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_C │ Valid non-expiring-soon token → 0 HTTP │
│ acheHit │ calls │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_C │ Cold cache → exactly 1 API call, cache │
│ acheMiss │ populated │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_E │ Hard-expired entry → triggers refresh │
│ xpiredToken │ │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_R │ Asserts the 5-min window at all boundary │
│ efreshAheadWindow (4 │ points │
│ sub-tests) │ │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_C │ 10 goroutines released simultaneously → │
│ oncurrentStampedePreventio │ exactly 1 API call │
│ ntest │ │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_C │ Different installation IDs each get their │
│ oncurrentDifferentInstalla │ own API call │
│ tions │ │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_R │ API 500 + stale cache → clear error │
│ efreshFailureSurfacesError │ returned, stale token NOT reused │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_R │ API 401, cold cache → clear error, empty │
│ efreshFailureNoCacheEntry │ token │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_N │ 404 → errors.Is(err, │
│ otFoundError │ ErrInstallationNotFound) │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_C │ Two installations maintain independent │
│ acheIsolation │ cache entries │
├────────────────────────────┼────────────────────────────────────────────┤
│ TestGetInstallationToken_S │ After warm-up, second call makes 0 │
│ econdCallUsesCache │ additional API calls │
└────────────────────────────┴────────────────────────────────────────────┘

──────────────────────────────────────────────────────────────────────────────

Test output

--- PASS: TestGetInstallationToken_CacheHit (0.11s)
--- PASS: TestGetInstallationToken_CacheMiss (0.14s)
--- PASS: TestGetInstallationToken_ExpiredToken (0.03s)
--- PASS: TestGetInstallationToken_RefreshAheadWindow (0.65s)
--- PASS: .../well_within_window_no_refresh
--- PASS: .../at_boundary_stale
--- PASS: .../inside_window_refresh
--- PASS: .../nearly_expired_refresh
--- PASS: TestGetInstallationToken_ConcurrentStampedePreventiontest (0.15s)
--- PASS: TestGetInstallationToken_ConcurrentDifferentInstallations (0.25s)
--- PASS: TestGetInstallationToken_RefreshFailureSurfacesError (0.12s)
--- PASS: TestGetInstallationToken_RefreshFailureNoCacheEntry (0.15s)
--- PASS: TestGetInstallationToken_NotFoundError (0.28s)
--- PASS: TestGetInstallationToken_CacheIsolation (0.13s)
--- PASS: TestCachedToken_IsValid (0.00s)
--- PASS: TestGetInstallationToken_SecondCallUsesCache (0.04s)
ok github.com/jagadeesh/grainlify/backend/internal/github 20.981s

All 32 tests in the package pass under -race.

──────────────────────────────────────────────────────────────────────────────

Security notes

  • An expired token being silently reused would cause GitHub API calls to fail
    with 401s in production
  • Without singleflight, a burst of concurrent callers observing a stale cache
    could itself trip GitHub's rate limits
  • Neither scenario can occur after this change: failures surface as errors,
    and concurrent refreshes are deduplicated

close #175

…l/github/app.go

- Add tokenRefreshAheadWindow const (5 min) with documented rationale
- Add cachedToken struct with isValid() boundary check
- Rework GetInstallationToken with in-memory cache + singleflight to
  prevent thundering-herd token-exchange stampedes under load
- Extract fetchInstallationToken helper returning (token, expiresAt, error)
- Initialize tokenCache map in NewGitHubAppClient

Tests added (13 new, all passing under -race):
- TestCachedToken_IsValid: boundary logic at all edges of the 5-min window
- TestGetInstallationToken_CacheHit: valid token reused, 0 HTTP calls
- TestGetInstallationToken_CacheMiss: cold cache triggers exactly 1 API call
- TestGetInstallationToken_ExpiredToken: expired entry triggers refresh
- TestGetInstallationToken_RefreshAheadWindow: asserts the 5-min window boundary
- TestGetInstallationToken_ConcurrentStampedePreventiontest: 10 concurrent
  callers coalesce into exactly 1 API call via singleflight
- TestGetInstallationToken_ConcurrentDifferentInstallations: distinct
  installations each get their own API call
- TestGetInstallationToken_RefreshFailureSurfacesError: error surfaces
  clearly; stale token is NOT silently reused
- TestGetInstallationToken_RefreshFailureNoCacheEntry: cold-cache failure path
- TestGetInstallationToken_NotFoundError: 404 maps to ErrInstallationNotFound
- TestGetInstallationToken_CacheIsolation: independent cache per installation
- TestGetInstallationToken_SecondCallUsesCache: warm cache, 0 extra API calls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit tests for GitHub App installation-token caching in internal/github/app.go

1 participant