Skip to content

Commit 8c11cca

Browse files
tstrombergtstromberg
committed
more elegance
1 parent c673ab2 commit 8c11cca

5 files changed

Lines changed: 151 additions & 71 deletions

File tree

‎cmd/prcost/main.go‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,9 @@ func main() {
182182
case "human":
183183
printHumanReadable(&breakdown, prURL)
184184
case "json":
185-
if err := printJSON(&breakdown); err != nil {
185+
encoder := json.NewEncoder(os.Stdout)
186+
encoder.SetIndent("", " ")
187+
if err := encoder.Encode(&breakdown); err != nil {
186188
log.Fatalf("Failed to output results: %v", err)
187189
}
188190
default:
@@ -321,7 +323,7 @@ func printDelayCosts(breakdown *cost.Breakdown, formatCurrency func(float64) str
321323
if breakdown.DelayCapped {
322324
cappedSuffix = " (capped)"
323325
}
324-
fmt.Printf(" Delivery %12s %s%s\n",
326+
fmt.Printf(" Project %12s %s%s\n",
325327
formatCurrency(breakdown.DelayCostDetail.DeliveryDelayCost),
326328
formatTimeUnit(breakdown.DelayCostDetail.DeliveryDelayHours),
327329
cappedSuffix)
@@ -425,10 +427,3 @@ func formatWithCommas(amount float64) string {
425427

426428
return string(result) + "." + decPart
427429
}
428-
429-
// printJSON outputs the cost breakdown in JSON format.
430-
func printJSON(breakdown *cost.Breakdown) error {
431-
encoder := json.NewEncoder(os.Stdout)
432-
encoder.SetIndent("", " ")
433-
return encoder.Encode(breakdown)
434-
}

‎cmd/prcost/repository.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"log/slog"
8+
"strconv"
89
"time"
910

1011
"github.com/codeGROOVE-dev/prcost/pkg/cost"
@@ -244,8 +245,10 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
244245
avgTotalCost := ext.TotalCost / float64(ext.TotalPRs)
245246
avgTotalHours := ext.TotalHours / float64(ext.TotalPRs)
246247

247-
// Show average PR breakdown
248-
fmt.Println(" Average PR (from sample):")
248+
// Show average PR breakdown with improved visual hierarchy
249+
fmt.Println(" ┌─────────────────────────────────────────────────────────────┐")
250+
fmt.Printf(" │ Average PR (sampled over %d day period)%*s│\n", days, 51-len(strconv.Itoa(days)), "")
251+
fmt.Println(" └─────────────────────────────────────────────────────────────┘")
249252
fmt.Println()
250253

251254
// Authors section
@@ -287,7 +290,7 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
287290
// Merge Delay section
288291
fmt.Println(" Delay Costs")
289292
fmt.Println(" ───────────")
290-
fmt.Printf(" Delivery %12s %s\n",
293+
fmt.Printf(" Project %12s %s\n",
291294
formatWithCommas(avgDeliveryDelayCost), formatTimeUnit(avgDeliveryDelayHours))
292295
fmt.Printf(" Coordination %12s %s\n",
293296
formatWithCommas(avgCoordinationCost), formatTimeUnit(avgCoordinationHours))
@@ -346,8 +349,11 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
346349
fmt.Println()
347350
fmt.Println()
348351

349-
// Extrapolated total section
350-
fmt.Printf(" Extrapolated Total (%d PRs):\n", ext.TotalPRs)
352+
// Extrapolated total section with improved visual hierarchy
353+
fmt.Println(" ┌─────────────────────────────────────────────────────────────┐")
354+
fmt.Printf(" │ Estimated Total Changes (extrapolated across %d day period)%*s│\n", days, 35-len(strconv.Itoa(days)), "")
355+
fmt.Printf(" │ Total PRs: %d%*s│\n", ext.TotalPRs, 60-len(strconv.Itoa(ext.TotalPRs)), "")
356+
fmt.Println(" └─────────────────────────────────────────────────────────────┘")
351357
fmt.Println()
352358

353359
// Authors section (extrapolated)
@@ -389,7 +395,7 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
389395
// Merge Delay section (extrapolated)
390396
fmt.Println(" Delay Costs")
391397
fmt.Println(" ───────────")
392-
fmt.Printf(" Delivery %12s %s\n",
398+
fmt.Printf(" Project %12s %s\n",
393399
formatWithCommas(ext.DeliveryDelayCost), formatTimeUnit(ext.DeliveryDelayHours))
394400
fmt.Printf(" Coordination %12s %s\n",
395401
formatWithCommas(ext.CoordinationCost), formatTimeUnit(ext.CoordinationHours))

‎internal/server/server.go‎

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ func New() *Server {
185185

186186
// Load GitHub token at startup and cache in memory for performance and billing.
187187
// This avoids repeated GSM API calls which cost money.
188-
token := server.getFallbackToken(ctx)
188+
token := server.token(ctx)
189189
if token != "" {
190190
logger.InfoContext(ctx, "GitHub fallback token loaded at startup")
191191
} else {
@@ -256,8 +256,8 @@ func (s *Server) SetDataSource(source string) {
256256
s.logger.InfoContext(ctx, "Data source configured", "source", source)
257257
}
258258

259-
// getLimiter returns a rate limiter for the given IP address.
260-
func (s *Server) getLimiter(ctx context.Context, ip string) *rate.Limiter {
259+
// limiter returns a rate limiter for the given IP address.
260+
func (s *Server) limiter(ctx context.Context, ip string) *rate.Limiter {
261261
s.ipLimitersMu.RLock()
262262
limiter, exists := s.ipLimiters[ip]
263263
s.ipLimitersMu.RUnlock()
@@ -325,8 +325,8 @@ func (s *Server) clearCache(mu *sync.RWMutex, cache map[string]*cacheEntry, name
325325
}
326326
}
327327

328-
// getCachedPRQuery retrieves cached PR query results.
329-
func (s *Server) getCachedPRQuery(key string) ([]github.PRSummary, bool) {
328+
// cachedPRQuery retrieves cached PR query results.
329+
func (s *Server) cachedPRQuery(key string) ([]github.PRSummary, bool) {
330330
s.prQueryCacheMu.RLock()
331331
defer s.prQueryCacheMu.RUnlock()
332332

@@ -349,8 +349,8 @@ func (s *Server) cachePRQuery(key string, prs []github.PRSummary) {
349349
}
350350
}
351351

352-
// getCachedPRData retrieves cached PR data.
353-
func (s *Server) getCachedPRData(key string) (cost.PRData, bool) {
352+
// cachedPRData retrieves cached PR data.
353+
func (s *Server) cachedPRData(key string) (cost.PRData, bool) {
354354
s.prDataCacheMu.RLock()
355355
defer s.prDataCacheMu.RUnlock()
356356

@@ -496,7 +496,9 @@ func (s *Server) handleCalculate(writer http.ResponseWriter, request *http.Reque
496496
ctx := request.Context()
497497

498498
// Extract client IP for rate limiting and logging.
499-
// Check X-Forwarded-For (only trust if behind known proxy).
499+
// SECURITY: X-Forwarded-For is trusted because Cloud Run (GCP) sanitizes it.
500+
// Cloud Run strips client-provided XFF headers and replaces with actual client IP.
501+
// For non-Cloud Run deployments, consider validating source or using RemoteAddr only.
500502
clientIP := request.RemoteAddr
501503
if xff := request.Header.Get("X-Forwarded-For"); xff != "" {
502504
if idx := strings.Index(xff, ","); idx > 0 {
@@ -512,7 +514,7 @@ func (s *Server) handleCalculate(writer http.ResponseWriter, request *http.Reque
512514
s.logger.InfoContext(ctx, "[handleCalculate] Incoming request", "client_ip", clientIP, "method", request.Method, "path", request.URL.Path)
513515

514516
// Per-IP rate limiting (SECURITY: Prevents single client from DoS-ing all users).
515-
limiter := s.getLimiter(ctx, clientIP)
517+
limiter := s.limiter(ctx, clientIP)
516518
if !limiter.Allow() {
517519
s.logger.WarnContext(ctx, "[handleCalculate] Rate limit exceeded", "client_ip", clientIP, "path", request.URL.Path)
518520
http.Error(writer, "Rate limit exceeded", http.StatusTooManyRequests)
@@ -531,7 +533,7 @@ func (s *Server) handleCalculate(writer http.ResponseWriter, request *http.Reque
531533
token := s.extractToken(request)
532534
if token == "" {
533535
// Try fallback token (GITHUB_TOKEN env var or GITHUB_SECRET from GSM)
534-
token = s.getFallbackToken(ctx)
536+
token = s.token(ctx)
535537
if token == "" {
536538
s.logger.WarnContext(ctx, "[handleCalculate] No GitHub token available", "remote_addr", request.RemoteAddr)
537539
http.Error(writer, "GitHub token required (set GITHUB_TOKEN env var or provide Authorization header)", http.StatusUnauthorized)
@@ -647,10 +649,10 @@ func (*Server) extractToken(r *http.Request) string {
647649
return auth
648650
}
649651

650-
// getFallbackToken retrieves a GitHub token from environment or Google Secret Manager.
652+
// token retrieves a GitHub token from environment or Google Secret Manager.
651653
// Results are cached in memory to avoid repeated API calls (performance and billing).
652654
// Priority: GITHUB_TOKEN env var, then GITHUB_TOKEN from GSM.
653-
func (s *Server) getFallbackToken(ctx context.Context) string {
655+
func (s *Server) token(ctx context.Context) string {
654656
// Check cache first (read lock)
655657
s.fallbackTokenMu.RLock()
656658
if s.fallbackToken != "" {
@@ -703,7 +705,7 @@ func (s *Server) processRequest(ctx context.Context, req *CalculateRequest, toke
703705

704706
// Try cache first
705707
cacheKey := fmt.Sprintf("pr:%s", req.URL)
706-
prData, cached := s.getCachedPRData(cacheKey)
708+
prData, cached := s.cachedPRData(cacheKey)
707709
if cached {
708710
s.logger.InfoContext(ctx, "[processRequest] Using cached PR data", "url", req.URL)
709711
} else {
@@ -923,6 +925,9 @@ func (s *Server) handleRepoSample(writer http.ResponseWriter, request *http.Requ
923925
ctx := request.Context()
924926

925927
// Extract client IP for rate limiting and logging.
928+
// SECURITY: X-Forwarded-For is trusted because Cloud Run (GCP) sanitizes it.
929+
// Cloud Run strips client-provided XFF headers and replaces with actual client IP.
930+
// For non-Cloud Run deployments, consider validating source or using RemoteAddr only.
926931
clientIP := request.RemoteAddr
927932
if xff := request.Header.Get("X-Forwarded-For"); xff != "" {
928933
if idx := strings.Index(xff, ","); idx > 0 {
@@ -938,7 +943,7 @@ func (s *Server) handleRepoSample(writer http.ResponseWriter, request *http.Requ
938943
s.logger.InfoContext(ctx, "[handleRepoSample] Incoming request", "client_ip", clientIP)
939944

940945
// Per-IP rate limiting.
941-
limiter := s.getLimiter(ctx, clientIP)
946+
limiter := s.limiter(ctx, clientIP)
942947
if !limiter.Allow() {
943948
s.logger.WarnContext(ctx, "[handleRepoSample] Rate limit exceeded", "client_ip", clientIP)
944949
http.Error(writer, "Rate limit exceeded", http.StatusTooManyRequests)
@@ -956,7 +961,7 @@ func (s *Server) handleRepoSample(writer http.ResponseWriter, request *http.Requ
956961
// Get auth token - try Authorization header first, then fallback.
957962
token := s.extractToken(request)
958963
if token == "" {
959-
token = s.getFallbackToken(ctx)
964+
token = s.token(ctx)
960965
if token == "" {
961966
s.logger.WarnContext(ctx, "[handleRepoSample] No GitHub token available", "remote_addr", request.RemoteAddr)
962967
http.Error(writer, "GitHub token required (set GITHUB_TOKEN env var or provide Authorization header)", http.StatusUnauthorized)
@@ -999,6 +1004,9 @@ func (s *Server) handleOrgSample(writer http.ResponseWriter, request *http.Reque
9991004
ctx := request.Context()
10001005

10011006
// Extract client IP for rate limiting and logging.
1007+
// SECURITY: X-Forwarded-For is trusted because Cloud Run (GCP) sanitizes it.
1008+
// Cloud Run strips client-provided XFF headers and replaces with actual client IP.
1009+
// For non-Cloud Run deployments, consider validating source or using RemoteAddr only.
10021010
clientIP := request.RemoteAddr
10031011
if xff := request.Header.Get("X-Forwarded-For"); xff != "" {
10041012
if idx := strings.Index(xff, ","); idx > 0 {
@@ -1014,7 +1022,7 @@ func (s *Server) handleOrgSample(writer http.ResponseWriter, request *http.Reque
10141022
s.logger.InfoContext(ctx, "[handleOrgSample] Incoming request", "client_ip", clientIP)
10151023

10161024
// Per-IP rate limiting.
1017-
limiter := s.getLimiter(ctx, clientIP)
1025+
limiter := s.limiter(ctx, clientIP)
10181026
if !limiter.Allow() {
10191027
s.logger.WarnContext(ctx, "[handleOrgSample] Rate limit exceeded", "client_ip", clientIP)
10201028
http.Error(writer, "Rate limit exceeded", http.StatusTooManyRequests)
@@ -1032,7 +1040,7 @@ func (s *Server) handleOrgSample(writer http.ResponseWriter, request *http.Reque
10321040
// Get auth token - try Authorization header first, then fallback.
10331041
token := s.extractToken(request)
10341042
if token == "" {
1035-
token = s.getFallbackToken(ctx)
1043+
token = s.token(ctx)
10361044
if token == "" {
10371045
s.logger.WarnContext(ctx, "[handleOrgSample] No GitHub token available", "remote_addr", request.RemoteAddr)
10381046
http.Error(writer, "GitHub token required (set GITHUB_TOKEN env var or provide Authorization header)", http.StatusUnauthorized)
@@ -1154,7 +1162,7 @@ func (s *Server) processRepoSample(ctx context.Context, req *RepoSampleRequest,
11541162

11551163
// Try cache first
11561164
cacheKey := fmt.Sprintf("repo:%s/%s:days=%d", req.Owner, req.Repo, req.Days)
1157-
prs, cached := s.getCachedPRQuery(cacheKey)
1165+
prs, cached := s.cachedPRQuery(cacheKey)
11581166
if cached {
11591167
s.logger.InfoContext(ctx, "Using cached PR query results",
11601168
"owner", req.Owner, "repo", req.Repo, "total_prs", len(prs))
@@ -1192,7 +1200,7 @@ func (s *Server) processRepoSample(ctx context.Context, req *RepoSampleRequest,
11921200

11931201
// Try cache first
11941202
prCacheKey := fmt.Sprintf("pr:%s", prURL)
1195-
prData, prCached := s.getCachedPRData(prCacheKey)
1203+
prData, prCached := s.cachedPRData(prCacheKey)
11961204
if !prCached {
11971205
var err error
11981206
// Use configured data source with updatedAt for effective caching
@@ -1241,7 +1249,7 @@ func (s *Server) processOrgSample(ctx context.Context, req *OrgSampleRequest, to
12411249

12421250
// Try cache first
12431251
cacheKey := fmt.Sprintf("org:%s:days=%d", req.Org, req.Days)
1244-
prs, cached := s.getCachedPRQuery(cacheKey)
1252+
prs, cached := s.cachedPRQuery(cacheKey)
12451253
if cached {
12461254
s.logger.InfoContext(ctx, "Using cached PR query results",
12471255
"org", req.Org, "total_prs", len(prs))
@@ -1278,7 +1286,7 @@ func (s *Server) processOrgSample(ctx context.Context, req *OrgSampleRequest, to
12781286

12791287
// Try cache first
12801288
prCacheKey := fmt.Sprintf("pr:%s", prURL)
1281-
prData, prCached := s.getCachedPRData(prCacheKey)
1289+
prData, prCached := s.cachedPRData(prCacheKey)
12821290
if !prCached {
12831291
var err error
12841292
// Use configured data source with updatedAt for effective caching
@@ -1368,6 +1376,9 @@ func (s *Server) handleRepoSampleStream(writer http.ResponseWriter, request *htt
13681376
ctx := request.Context()
13691377

13701378
// Extract client IP for rate limiting and logging.
1379+
// SECURITY: X-Forwarded-For is trusted because Cloud Run (GCP) sanitizes it.
1380+
// Cloud Run strips client-provided XFF headers and replaces with actual client IP.
1381+
// For non-Cloud Run deployments, consider validating source or using RemoteAddr only.
13711382
clientIP := request.RemoteAddr
13721383
if xff := request.Header.Get("X-Forwarded-For"); xff != "" {
13731384
if idx := strings.Index(xff, ","); idx > 0 {
@@ -1382,7 +1393,7 @@ func (s *Server) handleRepoSampleStream(writer http.ResponseWriter, request *htt
13821393
s.logger.InfoContext(ctx, "[handleRepoSampleStream] Incoming request", "client_ip", clientIP)
13831394

13841395
// Per-IP rate limiting.
1385-
limiter := s.getLimiter(ctx, clientIP)
1396+
limiter := s.limiter(ctx, clientIP)
13861397
if !limiter.Allow() {
13871398
s.logger.WarnContext(ctx, "[handleRepoSampleStream] Rate limit exceeded", "client_ip", clientIP)
13881399
http.Error(writer, "Rate limit exceeded", http.StatusTooManyRequests)
@@ -1401,7 +1412,7 @@ func (s *Server) handleRepoSampleStream(writer http.ResponseWriter, request *htt
14011412
// Get auth token - try Authorization header first, then fallback.
14021413
token := s.extractToken(request)
14031414
if token == "" {
1404-
token = s.getFallbackToken(ctx)
1415+
token = s.token(ctx)
14051416
if token == "" {
14061417
s.logger.WarnContext(ctx, "[handleRepoSampleStream] No GitHub token available", "remote_addr", request.RemoteAddr)
14071418
http.Error(writer, "GitHub token required (set GITHUB_TOKEN env var or provide Authorization header)", http.StatusUnauthorized)
@@ -1441,6 +1452,9 @@ func (s *Server) handleOrgSampleStream(writer http.ResponseWriter, request *http
14411452
ctx := request.Context()
14421453

14431454
// Extract client IP for rate limiting and logging.
1455+
// SECURITY: X-Forwarded-For is trusted because Cloud Run (GCP) sanitizes it.
1456+
// Cloud Run strips client-provided XFF headers and replaces with actual client IP.
1457+
// For non-Cloud Run deployments, consider validating source or using RemoteAddr only.
14441458
clientIP := request.RemoteAddr
14451459
if xff := request.Header.Get("X-Forwarded-For"); xff != "" {
14461460
if idx := strings.Index(xff, ","); idx > 0 {
@@ -1455,7 +1469,7 @@ func (s *Server) handleOrgSampleStream(writer http.ResponseWriter, request *http
14551469
s.logger.InfoContext(ctx, "[handleOrgSampleStream] Incoming request", "client_ip", clientIP)
14561470

14571471
// Per-IP rate limiting.
1458-
limiter := s.getLimiter(ctx, clientIP)
1472+
limiter := s.limiter(ctx, clientIP)
14591473
if !limiter.Allow() {
14601474
s.logger.WarnContext(ctx, "[handleOrgSampleStream] Rate limit exceeded", "client_ip", clientIP)
14611475
http.Error(writer, "Rate limit exceeded", http.StatusTooManyRequests)
@@ -1473,7 +1487,7 @@ func (s *Server) handleOrgSampleStream(writer http.ResponseWriter, request *http
14731487
// Get auth token - try Authorization header first, then fallback.
14741488
token := s.extractToken(request)
14751489
if token == "" {
1476-
token = s.getFallbackToken(ctx)
1490+
token = s.token(ctx)
14771491
if token == "" {
14781492
s.logger.WarnContext(ctx, "[handleOrgSampleStream] No GitHub token available", "remote_addr", request.RemoteAddr)
14791493
http.Error(writer, "GitHub token required (set GITHUB_TOKEN env var or provide Authorization header)", http.StatusUnauthorized)
@@ -1592,7 +1606,7 @@ func (s *Server) processRepoSampleWithProgress(ctx context.Context, req *RepoSam
15921606

15931607
// Try cache first
15941608
cacheKey := fmt.Sprintf("repo:%s/%s:days=%d", req.Owner, req.Repo, req.Days)
1595-
prs, cached := s.getCachedPRQuery(cacheKey)
1609+
prs, cached := s.cachedPRQuery(cacheKey)
15961610
if !cached {
15971611
// Send progress update before GraphQL query
15981612
logSSEError(ctx, s.logger, sendSSE(writer, ProgressUpdate{
@@ -1696,7 +1710,7 @@ func (s *Server) processOrgSampleWithProgress(ctx context.Context, req *OrgSampl
16961710

16971711
// Try cache first
16981712
cacheKey := fmt.Sprintf("org:%s:days=%d", req.Org, req.Days)
1699-
prs, cached := s.getCachedPRQuery(cacheKey)
1713+
prs, cached := s.cachedPRQuery(cacheKey)
17001714
if !cached {
17011715
// Send progress update before GraphQL query
17021716
logSSEError(ctx, s.logger, sendSSE(writer, ProgressUpdate{
@@ -1826,7 +1840,7 @@ func (s *Server) processPRsInParallel(workCtx, reqCtx context.Context, samples [
18261840

18271841
// Try cache first
18281842
prCacheKey := fmt.Sprintf("pr:%s", prURL)
1829-
prData, prCached := s.getCachedPRData(prCacheKey)
1843+
prData, prCached := s.cachedPRData(prCacheKey)
18301844
if !prCached {
18311845
var err error
18321846
// Use work context for actual API calls (not tied to client connection)

‎internal/server/server_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,7 @@ func TestRateLimiting(t *testing.T) {
491491
req1.RemoteAddr = "192.168.1.1:12345"
492492

493493
// Get rate limiter for this IP
494-
limiter := s.getLimiter(req1.Context(), "192.168.1.1")
494+
limiter := s.limiter(req1.Context(), "192.168.1.1")
495495

496496
// First request - allowed
497497
if !limiter.Allow() {

0 commit comments

Comments
 (0)