diff --git a/internal/github/app.go b/internal/github/app.go index 6291bc4..95ea148 100644 --- a/internal/github/app.go +++ b/internal/github/app.go @@ -11,9 +11,11 @@ import ( "net/http" "net/url" "strconv" + "sync" "time" "github.com/golang-jwt/jwt/v5" + "golang.org/x/sync/singleflight" ) // ErrInstallationNotFound is the sentinel error returned by GetInstallationToken when @@ -50,6 +52,29 @@ func (c realClock) Now() time.Time { return time.Now() } +// tokenRefreshAheadWindow is how far before a cached installation token's +// expiry we proactively fetch a fresh one. GitHub tokens last ~1 hour; +// refreshing 5 minutes early gives callers a comfortable buffer against +// clock-skew and in-flight request latency without hammering the API. +// +// Example: a token that expires at T=60m is treated as stale at T=55m +// and will be transparently replaced on the next call to GetInstallationToken. +const tokenRefreshAheadWindow = 5 * time.Minute + +// cachedToken holds a GitHub installation access token together with its +// expiry so the cache can decide when to refresh. +type cachedToken struct { + token string + expiresAt time.Time +} + +// isValid reports whether the token is still usable: it must not be empty +// and its expiry must be more than tokenRefreshAheadWindow away from now. +// Using the clock from the parent client keeps behaviour deterministic in tests. +func (ct cachedToken) isValid(now time.Time) bool { + return ct.token != "" && now.Before(ct.expiresAt.Add(-tokenRefreshAheadWindow)) +} + // GitHubAppClient handles GitHub App API calls type GitHubAppClient struct { AppID string @@ -58,6 +83,15 @@ type GitHubAppClient struct { UserAgent string BaseURL string Clock Clock + + // tokenCacheMu guards tokenCache. + tokenCacheMu sync.Mutex + // tokenCache maps installation ID → the most recently fetched token. + tokenCache map[string]cachedToken + // tokenFlight deduplicates concurrent token-exchange requests for the + // same installation ID, preventing a thundering-herd of GitHub API calls + // when many goroutines notice an expired token at the same time. + tokenFlight singleflight.Group } // NewGitHubAppClient creates a new GitHub App client @@ -84,6 +118,7 @@ func NewGitHubAppClient(appID string, privateKeyPEM string) (*GitHubAppClient, e UserAgent: "grainlify-backend", BaseURL: "https://api.github.com", Clock: realClock{}, + tokenCache: make(map[string]cachedToken), }, nil } @@ -120,17 +155,77 @@ type InstallationTokenResponse struct { ExpiresAt time.Time `json:"expires_at"` } -// GetInstallationToken gets an installation access token for a specific installation +// GetInstallationToken returns a valid installation access token for the given +// installation ID. Tokens are cached in memory and reused until they are +// within tokenRefreshAheadWindow of expiry (default: 5 minutes), at which +// point a new token is fetched from the GitHub API. +// +// Concurrent callers that all find the cached token stale are coalesced via +// a singleflight.Group so that only one token-exchange request is sent to +// GitHub, regardless of how many goroutines are waiting. All waiters receive +// the same fresh token. +// +// If the token-exchange call fails, the error is returned directly; no stale +// token is silently reused, because an expired token would cause downstream +// GitHub API calls to fail with 401s. func (c *GitHubAppClient) GetInstallationToken(ctx context.Context, installationID string) (string, error) { + // Fast path: return the cached token if it is still valid. + c.tokenCacheMu.Lock() + if c.tokenCache == nil { + c.tokenCache = make(map[string]cachedToken) + } + if ct, ok := c.tokenCache[installationID]; ok && ct.isValid(c.Clock.Now()) { + c.tokenCacheMu.Unlock() + return ct.token, nil + } + c.tokenCacheMu.Unlock() + + // Slow path: fetch a new token. Use singleflight so that N concurrent + // callers that all found the cache stale send exactly one HTTP request. + type result struct { + token string + expiresAt time.Time + } + v, err, _ := c.tokenFlight.Do(installationID, func() (interface{}, error) { + // Re-check the cache inside the singleflight callback: a previous + // waiter may have already populated it while we were waiting. + c.tokenCacheMu.Lock() + if ct, ok := c.tokenCache[installationID]; ok && ct.isValid(c.Clock.Now()) { + c.tokenCacheMu.Unlock() + return result{token: ct.token, expiresAt: ct.expiresAt}, nil + } + c.tokenCacheMu.Unlock() + + token, expiresAt, err := c.fetchInstallationToken(ctx, installationID) + if err != nil { + return nil, err + } + + c.tokenCacheMu.Lock() + c.tokenCache[installationID] = cachedToken{token: token, expiresAt: expiresAt} + c.tokenCacheMu.Unlock() + + return result{token: token, expiresAt: expiresAt}, nil + }) + if err != nil { + return "", err + } + return v.(result).token, nil +} + +// fetchInstallationToken performs the actual GitHub API call to exchange a +// signed App JWT for a short-lived installation access token. It returns +// the token string and its expiry time so the caller can populate the cache. +func (c *GitHubAppClient) fetchInstallationToken(ctx context.Context, installationID string) (string, time.Time, error) { jwtToken, err := c.GenerateJWT() if err != nil { - return "", fmt.Errorf("failed to generate JWT: %w", err) + return "", time.Time{}, fmt.Errorf("failed to generate JWT: %w", err) } url := fmt.Sprintf("%s/app/installations/%s/access_tokens", c.BaseURL, installationID) req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, nil) if err != nil { - return "", err + return "", time.Time{}, err } req.Header.Set("Authorization", "Bearer "+jwtToken) @@ -141,7 +236,7 @@ func (c *GitHubAppClient) GetInstallationToken(ctx context.Context, installation resp, err := c.HTTP.Do(req) if err != nil { - return "", err + return "", time.Time{}, err } defer resp.Body.Close() @@ -149,20 +244,20 @@ func (c *GitHubAppClient) GetInstallationToken(ctx context.Context, installation var errBody map[string]interface{} json.NewDecoder(resp.Body).Decode(&errBody) if resp.StatusCode == http.StatusNotFound { - return "", &InstallationNotFoundError{ + return "", time.Time{}, &InstallationNotFoundError{ InstallationID: installationID, StatusCode: resp.StatusCode, } } - return "", fmt.Errorf("failed to get installation token: status %d, error: %v", resp.StatusCode, errBody) + return "", time.Time{}, fmt.Errorf("failed to get installation token: status %d, error: %v", resp.StatusCode, errBody) } var tokenResp InstallationTokenResponse if err := json.NewDecoder(resp.Body).Decode(&tokenResp); err != nil { - return "", err + return "", time.Time{}, err } - return tokenResp.Token, nil + return tokenResp.Token, tokenResp.ExpiresAt, nil } // InstallationRepository represents a repository in a GitHub App installation diff --git a/internal/github/app_test.go b/internal/github/app_test.go index 3884524..db93e77 100644 --- a/internal/github/app_test.go +++ b/internal/github/app_test.go @@ -6,9 +6,13 @@ import ( "crypto/rsa" "encoding/base64" "encoding/json" + "errors" + "fmt" "net/http" "net/http/httptest" "strings" + "sync" + "sync/atomic" "testing" "time" @@ -651,3 +655,625 @@ func TestListInstallationRepositories_ContextCancellation(t *testing.T) { func strPtr(s string) *string { return &s } + +// --------------------------------------------------------------------------- +// Installation-token caching tests +// --------------------------------------------------------------------------- +// +// These tests exercise the four observable behaviours of the cache layer that +// lives inside GetInstallationToken: +// +// 1. Cache HIT – a valid, non-expiring-soon cached token is returned +// without making any HTTP request. +// 2. Cache MISS / expiry – an absent or expired token triggers exactly one +// HTTP round-trip; the fresh token is returned. +// 3. Refresh-ahead window – a token whose expiry is within +// tokenRefreshAheadWindow (5 min) of "now" is treated as +// stale and exchanged, even though it hasn't expired yet. +// 4. Stampede prevention – N concurrent callers that all observe a stale +// cache coalesce into exactly ONE GitHub API request. +// 5. Refresh failure – when the GitHub API returns an error the caller +// receives a clear error; no stale/expired token is silently +// returned. + +// --------------------------------------------------------------------------- +// helpers +// --------------------------------------------------------------------------- + +// newTokenExpiresAt returns the string form of t.Add(d) suitable for use as +// the "expires_at" field in a mock GitHub response body. +func newTokenExpiresAt(t *testing.T, base time.Time, d time.Duration) string { + t.Helper() + return base.Add(d).UTC().Format(time.RFC3339) +} + +// buildTokenClient creates a GitHubAppClient whose HTTP transport points at +// the provided httptest.Server and whose Clock is fixed at clockNow. +func buildTokenClient(t *testing.T, server *httptest.Server, clockNow time.Time) *GitHubAppClient { + t.Helper() + privateKey, _ := generateTestRSAKey(t) + return &GitHubAppClient{ + AppID: "test-app", + PrivateKey: privateKey, + HTTP: server.Client(), + UserAgent: "test", + BaseURL: server.URL, + Clock: mockClock{now: clockNow}, + tokenCache: make(map[string]cachedToken), + } +} + +// tokenResponse writes a standard GitHub installation-token JSON response. +func tokenResponse(w http.ResponseWriter, token, expiresAt string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(map[string]interface{}{ + "token": token, + "expires_at": expiresAt, + }) +} + +// --------------------------------------------------------------------------- +// 1. Cache hit +// --------------------------------------------------------------------------- + +// TestGetInstallationToken_CacheHit asserts that a valid, non-expiring-soon +// cached token is returned without performing any HTTP request. +func TestGetInstallationToken_CacheHit(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + // The server should never be called; fail loudly if it is. + callCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + t.Errorf("unexpected HTTP call to token endpoint (cache should have served the request)") + tokenResponse(w, "ghs_should_not_be_called", newTokenExpiresAt(t, now, time.Hour)) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + // Pre-populate the cache with a token that expires 30 minutes from now — + // well outside the 5-minute refresh-ahead window. + cachedTok := "ghs_cached_valid_token" + client.tokenCacheMu.Lock() + client.tokenCache["inst-1"] = cachedToken{ + token: cachedTok, + expiresAt: now.Add(30 * time.Minute), + } + client.tokenCacheMu.Unlock() + + got, err := client.GetInstallationToken(context.Background(), "inst-1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != cachedTok { + t.Errorf("expected cached token %q, got %q", cachedTok, got) + } + if callCount != 0 { + t.Errorf("expected 0 HTTP calls, got %d", callCount) + } +} + +// --------------------------------------------------------------------------- +// 2. Cache miss – no entry yet +// --------------------------------------------------------------------------- + +// TestGetInstallationToken_CacheMiss asserts that when there is no cached +// token exactly one HTTP request is made and the returned token is stored. +func TestGetInstallationToken_CacheMiss(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + callCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + tokenResponse(w, "ghs_fresh_token", newTokenExpiresAt(t, now, time.Hour)) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + got, err := client.GetInstallationToken(context.Background(), "inst-2") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "ghs_fresh_token" { + t.Errorf("expected %q, got %q", "ghs_fresh_token", got) + } + if callCount != 1 { + t.Errorf("expected exactly 1 HTTP call, got %d", callCount) + } + + // The token should now be in the cache. + client.tokenCacheMu.Lock() + ct := client.tokenCache["inst-2"] + client.tokenCacheMu.Unlock() + if ct.token != "ghs_fresh_token" { + t.Errorf("cache not populated: expected %q, got %q", "ghs_fresh_token", ct.token) + } +} + +// --------------------------------------------------------------------------- +// 3a. Expired token → refresh +// --------------------------------------------------------------------------- + +// TestGetInstallationToken_ExpiredToken asserts that a token whose expiry has +// already passed (hard-expired) triggers a fresh API call. +func TestGetInstallationToken_ExpiredToken(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + callCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + tokenResponse(w, "ghs_refreshed_token", newTokenExpiresAt(t, now, time.Hour)) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + // Inject an already-expired token. + client.tokenCacheMu.Lock() + client.tokenCache["inst-3"] = cachedToken{ + token: "ghs_expired_token", + expiresAt: now.Add(-10 * time.Minute), // expired 10 minutes ago + } + client.tokenCacheMu.Unlock() + + got, err := client.GetInstallationToken(context.Background(), "inst-3") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "ghs_refreshed_token" { + t.Errorf("expected refreshed token %q, got %q", "ghs_refreshed_token", got) + } + if callCount != 1 { + t.Errorf("expected exactly 1 HTTP call, got %d", callCount) + } +} + +// --------------------------------------------------------------------------- +// 3b. Refresh-ahead window +// --------------------------------------------------------------------------- + +// TestGetInstallationToken_RefreshAheadWindow asserts that the refresh-ahead +// window is exactly tokenRefreshAheadWindow (5 minutes). +// +// Tokens are refreshed when: now >= expiresAt - tokenRefreshAheadWindow +// i.e. when remaining TTL < tokenRefreshAheadWindow. +// +// We test three boundary points: +// - remaining = tokenRefreshAheadWindow + 1s → cache hit (still valid) +// - remaining = tokenRefreshAheadWindow → boundary, treated as stale +// - remaining = tokenRefreshAheadWindow - 1s → definitely stale, refresh +func TestGetInstallationToken_RefreshAheadWindow(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + cases := []struct { + name string + remaining time.Duration // time until token expires relative to now + expectFetch bool // true if we expect an HTTP call + }{ + { + name: "well_within_window_no_refresh", + remaining: tokenRefreshAheadWindow + time.Second, + expectFetch: false, + }, + { + name: "at_boundary_stale", + remaining: tokenRefreshAheadWindow, + expectFetch: true, + }, + { + name: "inside_window_refresh", + remaining: tokenRefreshAheadWindow - time.Second, + expectFetch: true, + }, + { + name: "nearly_expired_refresh", + remaining: 30 * time.Second, + expectFetch: true, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + callCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + tokenResponse(w, "ghs_new_token", newTokenExpiresAt(t, now, time.Hour)) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + // Seed the cache with a token that has the desired remaining TTL. + client.tokenCacheMu.Lock() + client.tokenCache["inst-window"] = cachedToken{ + token: "ghs_old_token", + expiresAt: now.Add(tc.remaining), + } + client.tokenCacheMu.Unlock() + + tok, err := client.GetInstallationToken(context.Background(), "inst-window") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if tc.expectFetch { + if callCount != 1 { + t.Errorf("expected 1 HTTP call (refresh), got %d", callCount) + } + if tok != "ghs_new_token" { + t.Errorf("expected fresh token, got %q", tok) + } + } else { + if callCount != 0 { + t.Errorf("expected 0 HTTP calls (cache hit), got %d", callCount) + } + if tok != "ghs_old_token" { + t.Errorf("expected cached token, got %q", tok) + } + } + }) + } +} + +// --------------------------------------------------------------------------- +// 4. Stampede prevention (concurrent callers) +// --------------------------------------------------------------------------- + +// TestGetInstallationToken_ConcurrentStampedePreventiontest verifies that N +// concurrent goroutines that all find an empty cache coalesce into exactly ONE +// HTTP call to the GitHub token endpoint (singleflight deduplication). +func TestGetInstallationToken_ConcurrentStampedePreventiontest(t *testing.T) { + const numCallers = 10 + + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + // Use an atomic counter to count how many times the handler is invoked. + var apiCallCount int64 + // Barrier to make all goroutines hit the client at the same moment. + var ready sync.WaitGroup + var release = make(chan struct{}) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt64(&apiCallCount, 1) + // Simulate a modest network delay so concurrent goroutines have time + // to queue up inside singleflight before the first one returns. + time.Sleep(20 * time.Millisecond) + tokenResponse(w, "ghs_singleton_token", newTokenExpiresAt(t, now, time.Hour)) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + results := make([]string, numCallers) + errs := make([]error, numCallers) + ready.Add(numCallers) + + var wg sync.WaitGroup + wg.Add(numCallers) + for i := 0; i < numCallers; i++ { + i := i + go func() { + defer wg.Done() + ready.Done() + <-release // wait for the signal to start simultaneously + results[i], errs[i] = client.GetInstallationToken(context.Background(), "inst-stampede") + }() + } + + // Wait until all goroutines are parked, then release them together. + ready.Wait() + close(release) + wg.Wait() + + // All callers must have received the token without error. + for i, err := range errs { + if err != nil { + t.Errorf("goroutine %d got error: %v", i, err) + } + } + for i, tok := range results { + if tok != "ghs_singleton_token" { + t.Errorf("goroutine %d got unexpected token %q", i, tok) + } + } + + // The critical assertion: only ONE API call must have reached the server. + if n := atomic.LoadInt64(&apiCallCount); n != 1 { + t.Errorf("singleflight failed: expected 1 API call, got %d", n) + } +} + +// TestGetInstallationToken_ConcurrentDifferentInstallations asserts that +// concurrent refreshes for DIFFERENT installation IDs each get their own +// API call (singleflight key is per installation ID). +func TestGetInstallationToken_ConcurrentDifferentInstallations(t *testing.T) { + const numInstallations = 5 + + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + var apiCallCount int64 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt64(&apiCallCount, 1) + // Extract installation ID from path to return a distinct token. + parts := strings.Split(r.URL.Path, "/") + instID := parts[len(parts)-2] + time.Sleep(10 * time.Millisecond) + tokenResponse(w, "ghs_token_"+instID, newTokenExpiresAt(t, now, time.Hour)) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + var wg sync.WaitGroup + results := make([]string, numInstallations) + errs := make([]error, numInstallations) + for i := 0; i < numInstallations; i++ { + wg.Add(1) + i := i + go func() { + defer wg.Done() + instID := fmt.Sprintf("inst-%d", i) + results[i], errs[i] = client.GetInstallationToken(context.Background(), instID) + }() + } + wg.Wait() + + for i, err := range errs { + if err != nil { + t.Errorf("installation %d error: %v", i, err) + } + wantTok := fmt.Sprintf("ghs_token_inst-%d", i) + if results[i] != wantTok { + t.Errorf("installation %d: expected %q, got %q", i, wantTok, results[i]) + } + } + + // Each installation should have triggered exactly one API call. + if n := atomic.LoadInt64(&apiCallCount); n != numInstallations { + t.Errorf("expected %d API calls (one per installation), got %d", numInstallations, n) + } +} + +// --------------------------------------------------------------------------- +// 5. Refresh failure — error surfaces clearly, stale token NOT reused +// --------------------------------------------------------------------------- + +// TestGetInstallationToken_RefreshFailureSurfacesError asserts that when the +// GitHub API returns a non-2xx response the caller receives a meaningful +// error, and no stale token is silently returned. +func TestGetInstallationToken_RefreshFailureSurfacesError(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + json.NewEncoder(w).Encode(map[string]interface{}{ + "message": "internal server error", + }) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + // Seed an expired stale token to confirm it is not returned on error. + client.tokenCacheMu.Lock() + client.tokenCache["inst-fail"] = cachedToken{ + token: "ghs_stale_token", + expiresAt: now.Add(-time.Hour), // hard-expired + } + client.tokenCacheMu.Unlock() + + _, err := client.GetInstallationToken(context.Background(), "inst-fail") + if err == nil { + t.Fatal("expected an error when the API call fails, got nil") + } + if !strings.Contains(err.Error(), "500") { + t.Errorf("expected error to mention HTTP status 500, got: %v", err) + } +} + +// TestGetInstallationToken_RefreshFailureNoCacheEntry asserts that a failure +// on a cold cache (no entry at all) returns an error rather than an empty +// token string. +func TestGetInstallationToken_RefreshFailureNoCacheEntry(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + json.NewEncoder(w).Encode(map[string]interface{}{"message": "Bad credentials"}) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + tok, err := client.GetInstallationToken(context.Background(), "inst-cold-fail") + if err == nil { + t.Fatalf("expected error, got token %q", tok) + } + if tok != "" { + t.Errorf("expected empty token on failure, got %q", tok) + } +} + +// TestGetInstallationToken_NotFoundError asserts that a 404 response returns +// the typed ErrInstallationNotFound sentinel. +func TestGetInstallationToken_NotFoundError(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + json.NewEncoder(w).Encode(map[string]interface{}{"message": "Not Found"}) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + _, err := client.GetInstallationToken(context.Background(), "inst-gone") + if err == nil { + t.Fatal("expected ErrInstallationNotFound, got nil") + } + if !errors.Is(err, ErrInstallationNotFound) { + t.Errorf("expected errors.Is(err, ErrInstallationNotFound), got: %v", err) + } +} + +// --------------------------------------------------------------------------- +// 6. Cache isolation between installations +// --------------------------------------------------------------------------- + +// TestGetInstallationToken_CacheIsolation asserts that two installations each +// maintain independent cache entries and independent token values. +func TestGetInstallationToken_CacheIsolation(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + parts := strings.Split(r.URL.Path, "/") + instID := parts[len(parts)-2] + tokenResponse(w, "ghs_token_for_"+instID, newTokenExpiresAt(t, now, time.Hour)) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + tokA, err := client.GetInstallationToken(context.Background(), "inst-A") + if err != nil { + t.Fatalf("inst-A error: %v", err) + } + tokB, err := client.GetInstallationToken(context.Background(), "inst-B") + if err != nil { + t.Fatalf("inst-B error: %v", err) + } + + if tokA == tokB { + t.Errorf("expected distinct tokens for distinct installations, both returned %q", tokA) + } + if tokA != "ghs_token_for_inst-A" { + t.Errorf("inst-A: expected %q, got %q", "ghs_token_for_inst-A", tokA) + } + if tokB != "ghs_token_for_inst-B" { + t.Errorf("inst-B: expected %q, got %q", "ghs_token_for_inst-B", tokB) + } + + // Second call for inst-A must come from cache (no extra HTTP call). + callCount := 0 + // We can't intercept only the second call via the existing server, so we + // assert indirectly: the cached value must match what was returned earlier. + client.tokenCacheMu.Lock() + ctA := client.tokenCache["inst-A"] + ctB := client.tokenCache["inst-B"] + client.tokenCacheMu.Unlock() + + _ = callCount + if ctA.token != tokA { + t.Errorf("cache for inst-A mismatch: %q vs %q", ctA.token, tokA) + } + if ctB.token != tokB { + t.Errorf("cache for inst-B mismatch: %q vs %q", ctB.token, tokB) + } +} + +// --------------------------------------------------------------------------- +// 7. cachedToken.isValid unit tests +// --------------------------------------------------------------------------- + +// TestCachedToken_IsValid directly exercises the isValid boundary logic to +// ensure the refresh-ahead window is asserted at the unit level. +func TestCachedToken_IsValid(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + cases := []struct { + name string + expiresAt time.Time + want bool + }{ + { + name: "empty_token", + expiresAt: now.Add(time.Hour), + want: false, // empty token string → invalid regardless of expiry + }, + { + name: "far_future", + expiresAt: now.Add(time.Hour), + want: true, + }, + { + name: "just_outside_refresh_window", + expiresAt: now.Add(tokenRefreshAheadWindow + time.Second), + want: true, + }, + { + name: "at_refresh_window_boundary", + expiresAt: now.Add(tokenRefreshAheadWindow), + want: false, + }, + { + name: "inside_refresh_window", + expiresAt: now.Add(tokenRefreshAheadWindow - time.Second), + want: false, + }, + { + name: "already_expired", + expiresAt: now.Add(-time.Minute), + want: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + token := "ghs_test" + if tc.name == "empty_token" { + token = "" + } + ct := cachedToken{token: token, expiresAt: tc.expiresAt} + got := ct.isValid(now) + if got != tc.want { + t.Errorf("isValid(%v): expected %v, got %v (expiresAt=%v, window=%v)", + tc.name, tc.want, got, tc.expiresAt, tokenRefreshAheadWindow) + } + }) + } +} + +// --------------------------------------------------------------------------- +// 8. Second call uses cache (no redundant API calls after warm-up) +// --------------------------------------------------------------------------- + +// TestGetInstallationToken_SecondCallUsesCache verifies that the second call +// for the same installation hits the cache and makes no additional API calls. +func TestGetInstallationToken_SecondCallUsesCache(t *testing.T) { + now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + + callCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + tokenResponse(w, "ghs_only_once", newTokenExpiresAt(t, now, time.Hour)) + })) + defer server.Close() + + client := buildTokenClient(t, server, now) + + // First call populates the cache. + tok1, err := client.GetInstallationToken(context.Background(), "inst-reuse") + if err != nil { + t.Fatalf("first call error: %v", err) + } + + // Second call should serve from cache. + tok2, err := client.GetInstallationToken(context.Background(), "inst-reuse") + if err != nil { + t.Fatalf("second call error: %v", err) + } + + if tok1 != tok2 { + t.Errorf("tokens should be identical: %q vs %q", tok1, tok2) + } + if callCount != 1 { + t.Errorf("expected exactly 1 API call, got %d", callCount) + } +}