fix add a regression test confirming different admission keys

Author: andriypolandki

src/github/graphql-cache.ts

@@ -66,6 +66,10 @@ async function graphqlCacheKey(query: string, token: string): Promise<string> {
   return `gql:v1:${authHash}:${queryHash}`;
 }
 
+function graphqlSingleFlightKey(cacheKey: string, admissionKey?: GitHubRateLimitAdmissionKey): string {
+  return `${cacheKey}:${admissionKey ?? ""}`;
+}
+
 function recordGraphQlCacheMetric(result: "hit" | "miss" | "set" | "coalesced" | "bypassed" | "error", cls: string): void {
   incr(GITHUB_GRAPHQL_CACHE_METRIC, { result, class: cls });
 }
@@ -152,7 +156,8 @@ export async function fetchCachedGitHubGraphQl(
   }
   recordGraphQlCacheMetric("miss", cls);
 
-  const existing = inFlightGraphQlPosts.get(cacheKey);
+  const singleFlightKey = graphqlSingleFlightKey(cacheKey, admissionKey);
+  const existing = inFlightGraphQlPosts.get(singleFlightKey);
   if (existing) {
     recordGraphQlCacheMetric("coalesced", cls);
     const replay = await existing;
@@ -164,8 +169,8 @@ export async function fetchCachedGitHubGraphQl(
     (error: unknown) => ({ ok: false as const, error }),
   );
   const shared = request.then((settled) => (settled.ok ? settled.result.cached : null));
-  const sharedWithCleanup = shared.finally(() => inFlightGraphQlPosts.delete(cacheKey));
-  inFlightGraphQlPosts.set(cacheKey, sharedWithCleanup);
+  const sharedWithCleanup = shared.finally(() => inFlightGraphQlPosts.delete(singleFlightKey));
+  inFlightGraphQlPosts.set(singleFlightKey, sharedWithCleanup);
   const result = await request;
   if (!result.ok) throw result.error;
   return result.result.response;

test/unit/github-graphql-cache.test.ts

@@ -194,6 +194,32 @@ describe("fetchCachedGitHubGraphQl", () => {
     expect(b.headers.get(GITHUB_RESPONSE_CACHE_REPLAY_HEADER)).toBe("coalesced");
   });
 
+  it("does not coalesce concurrent cold misses when admission keys differ", async () => {
+    installMemoryResponseCache();
+    const keyA = githubRateLimitAdmissionKeyForInstallation(111);
+    const keyB = githubRateLimitAdmissionKeyForInstallation(222);
+    let fetches = 0;
+    vi.stubGlobal("fetch", async () => {
+      fetches += 1;
+      await new Promise((resolve) => setTimeout(resolve, 20));
+      return Response.json(
+        { data: { repository: { issues: { totalCount: 1 } } } },
+        { headers: { "x-ratelimit-remaining": String(5000 - fetches), "x-ratelimit-reset": "1782802800" } },
+      );
+    });
+
+    const [a, b] = await Promise.all([
+      fetchCachedGitHubGraphQl(TOTALS_QUERY, "token-a", keyA),
+      fetchCachedGitHubGraphQl(TOTALS_QUERY, "token-a", keyB),
+    ]);
+
+    expect(fetches).toBe(2);
+    expect(a.headers.get(GITHUB_RESPONSE_CACHE_REPLAY_HEADER)).toBeNull();
+    expect(b.headers.get(GITHUB_RESPONSE_CACHE_REPLAY_HEADER)).toBeNull();
+    expect(latestGitHubRestRateLimitObservation(keyA)).not.toBeNull();
+    expect(latestGitHubRestRateLimitObservation(keyB)).not.toBeNull();
+  });
+
   it("bypasses cache for mutable PR detail queries", async () => {
     installMemoryResponseCache();
     let fetches = 0;