feat: add unacked message counts#29
Conversation
Test & Lint Summary
Tests: passed: 58, failed: 0, skipped: 0 |
noa-lucent
left a comment
noa-lucent
left a comment
There was a problem hiding this comment.
Implements Threads.GetUnackedMessageCounts per agynio/architecture#142 with a store-level grouped count query and self-only authorization at the handler boundary. Unit coverage exercises both auth and mapping.
Minor inline nits left:
- clarify the int32 overflow constant
- remove redundant
count <= 0filtering (COUNT(*) can’t yield <= 0)
(Static review only; I did not run tests locally.)
| if err := rows.Scan(&threadID, &count); err != nil { | ||
| return nil, err | ||
| } | ||
| if count > int64(^uint32(0)>>1) { |
There was a problem hiding this comment.
[nit] int64(^uint32(0)>>1) is a pretty opaque way to express max int32. Prefer a named const (e.g. const maxInt32 = int64(math.MaxInt32)) so the intent is obvious.
| } | ||
| resp := &threadsv1.GetUnackedMessageCountsResponse{CountsByThreadId: make(map[string]int32, len(counts))} | ||
| for threadID, count := range counts { | ||
| if count <= 0 { |
There was a problem hiding this comment.
[nit] COUNT(*) in the store query can never yield <= 0, so the if count <= 0 { continue } branch is redundant and slightly masks invariant violations. I’d either remove it (and just return the map) or treat unexpected values as an internal error.
3 participants
Summary
Testing
Related: agynio/architecture#142
#142