perf: cache metrics api requests with 1 hour revalidation

[YashKrTripathi]

Description

This Pull Request implements a Next.js caching strategy (next: { revalidate: 3600 }) across the main GitHub metrics endpoints to optimize dashboard response times and conserve GitHub API rate limits.

This PR was split from #269 as requested by the mentor to allow for a dedicated discussion on the caching trade-offs.

Key Changes

• Replaced cache: "no-store" with next: { revalidate: 3600 } (1-hour TTL) across the following metrics API endpoints:
  • src/lib/github.ts (issues, month counts)
  • src/app/api/metrics/contributions/route.ts
  • src/app/api/metrics/languages/route.ts
  • src/app/api/metrics/pinned-repos/route.ts
  • src/app/api/metrics/pr-breakdown/route.ts
  • src/app/api/metrics/prs/route.ts
  • src/app/api/metrics/repo-health/route.ts
  • src/app/api/metrics/repos/route.ts
  • src/app/api/metrics/streak/route.ts
  • src/app/api/metrics/weekly-summary/route.ts

Trade-offs / Discussion

Using cache: "no-store" causes fresh upstream requests to the GitHub API on every single component mount, which can exhaust the user's GitHub API rate limits very quickly under active usage. Adding a 1-hour revalidation window mitigates this rate-limit issue while keeping the dashboard data reasonably fresh.

[vercel]

@YashKrTripathi is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel.

A member of the Team first needs to authorize it.

[YashKrTripathi]

Hi! @Priyanshu-byte-coder ,
This PR has been split off from PR #269 (feat: implement public profile copy link button) to isolate the caching changes.

This PR (#362 — perf: cache metrics api requests with 1 hour revalidation) contains the caching strategy changes to help prevent GitHub API rate limit issues on the dashboard metrics.

Please let me know your thoughts on the caching trade-offs here. Once approved, please label it with the gssoc:approved tag. Thank you!

[Priyanshu-byte-coder]

This approach has a security vulnerability and cannot be merged as-is.

Cross-user data leak via shared fetch cache

Next.js fetch caching uses the URL as the cache key — it does not include request headers like Authorization. When two users call the same endpoint:

1. User A requests /api/metrics/prs → Next.js fetches https://api.github.com/search/issues?q=type:pr+author:@me&per_page=100 with User A's token, caches the response under that URL key.
2. User B requests /api/metrics/prs → Next.js finds a cached response for the same URL → returns User A's PR data to User B.

This affects every route using a static-ish URL:

• prs/route.ts: author:@me — same URL for all users
• pinned-repos/route.ts: GraphQL endpoint (/graphql) — same URL for all users
• pr-breakdown/route.ts: same issue

The correct approach for authenticated caching is a server-side cache keyed by githubId (e.g., Upstash Redis as in PR #311, or a server-side Map). Do not use next: { revalidate } for authenticated GitHub API requests.

[YashKrTripathi]

This approach has a security vulnerability and cannot be merged as-is.

Cross-user data leak via shared fetch cache

Next.js fetch caching uses the URL as the cache key — it does not include request headers like Authorization. When two users call the same endpoint:

1. User A requests /api/metrics/prs → Next.js fetches https://api.github.com/search/issues?q=type:pr+author:@me&per_page=100 with User A's token, caches the response under that URL key.
2. User B requests /api/metrics/prs → Next.js finds a cached response for the same URL → returns User A's PR data to User B.

This affects every route using a static-ish URL:

• prs/route.ts: author:@me — same URL for all users
• pinned-repos/route.ts: GraphQL endpoint (/graphql) — same URL for all users
• pr-breakdown/route.ts: same issue

The correct approach for authenticated caching is a server-side cache keyed by githubId (e.g., Upstash Redis as in PR #311, or a server-side Map). Do not use next: { revalidate } for authenticated GitHub API requests.

Sorry for the inconvenience caused will try asap to fix this issue

[YashKrTripathi]

@Priyanshu-byte-coder I am working on this pr until then please close the other one the copy button pr with the gssoc:aprooved tag pls

[YashKrTripathi]

This approach has a security vulnerability and cannot be merged as-is.

Cross-user data leak via shared fetch cache

Next.js fetch caching uses the URL as the cache key — it does not include request headers like Authorization. When two users call the same endpoint:

1. User A requests /api/metrics/prs → Next.js fetches https://api.github.com/search/issues?q=type:pr+author:@me&per_page=100 with User A's token, caches the response under that URL key.
2. User B requests /api/metrics/prs → Next.js finds a cached response for the same URL → returns User A's PR data to User B.

This affects every route using a static-ish URL:

• prs/route.ts: author:@me — same URL for all users
• pinned-repos/route.ts: GraphQL endpoint (/graphql) — same URL for all users
• pr-breakdown/route.ts: same issue

The correct approach for authenticated caching is a server-side cache keyed by githubId (e.g., Upstash Redis as in PR #311, or a server-side Map). Do not use next: { revalidate } for authenticated GitHub API requests.

@Priyanshu-byte-coder
Hi! I have successfully updated this PR to resolve the security vulnerability.

As requested, the feature set has been split into two separate PRs:

1. PR feat: implement public profile copy link button #269 (feat: implement public profile copy link button) — Implements the Copy Link Button feature.
2. PR perf: cache metrics api requests with 1 hour revalidation #362 (perf: cache metrics api requests with 1 hour revalidation) — Implements this secure, server-side in-memory caching.

Please close the duplicate PR #269 and merge this PR (#362). Once merged/approved, please apply the gssoc:approved tag. Thanks!

[YashKrTripathi]

@Priyanshu-byte-coder i am resolving conflicts please wait

[Priyanshu-byte-coder]

The token-hashed cache key approach (getCacheKey(token, namespace)) is correct — no cross-user data leaks. However, PR #311 (Upstash Redis caching) was just merged and already adds per-user caching for the same routes (streak, contributions, prs, repos) using Redis with proper TTLs.

Merging this PR would create two competing caching layers for the same data, with different TTLs and no coordination. When the Redis cache is empty (cold start, expired, or bypass), the in-memory cache could serve stale data from a previous request. When the in-memory cache is cleared (cold start), the overhead is pointless.

Please close this PR — the caching problem is solved by #311. If you want to improve the in-memory cache (e.g., as a faster L1 cache in front of Redis), open a new focused PR that integrates with the existing withMetricsCache interface from src/lib/metrics-cache.ts.

[YashKrTripathi]

The token-hashed cache key approach (getCacheKey(token, namespace)) is correct — no cross-user data leaks. However, PR #311 (Upstash Redis caching) was just merged and already adds per-user caching for the same routes (streak, contributions, prs, repos) using Redis with proper TTLs.

Merging this PR would create two competing caching layers for the same data, with different TTLs and no coordination. When the Redis cache is empty (cold start, expired, or bypass), the in-memory cache could serve stale data from a previous request. When the in-memory cache is cleared (cold start), the overhead is pointless.

Please close this PR — the caching problem is solved by #311. If you want to improve the in-memory cache (e.g., as a faster L1 cache in front of Redis), open a new focused PR that integrates with the existing withMetricsCache interface from src/lib/metrics-cache.ts.

yes raising this new issue asap please wait sir and assign that issue to me i am already in the project i can do that right now

[YashKrTripathi]

The token-hashed cache key approach (getCacheKey(token, namespace)) is correct — no cross-user data leaks. However, PR #311 (Upstash Redis caching) was just merged and already adds per-user caching for the same routes (streak, contributions, prs, repos) using Redis with proper TTLs.

Merging this PR would create two competing caching layers for the same data, with different TTLs and no coordination. When the Redis cache is empty (cold start, expired, or bypass), the in-memory cache could serve stale data from a previous request. When the in-memory cache is cleared (cold start), the overhead is pointless.

Please close this PR — the caching problem is solved by #311. If you want to improve the in-memory cache (e.g., as a faster L1 cache in front of Redis), open a new focused PR that integrates with the existing withMetricsCache interface from src/lib/metrics-cache.ts.

@Priyanshu-byte-coder Thanks for the clarification. I’ve opened a new focused PR: perf: implement local L1 memory cache in front of Upstash Redis (#400) that aligns with the existing caching architecture. Since PR #311 already solves the original caching issue, I’m closing this PR to avoid duplicate caching layers and potential inconsistencies.