perf(api): push social query filtering + pagination into the DB - #344
Conversation
GetFriendsQuery and GetCheersQuery materialized a user's full friendship/cheer history, then filtered blocked users and capped in memory. Add ISocialGraphReader (Infrastructure) so the blocked-user anti-join, ordering, lookback window, and row cap run server-side. - GetFriends: blocked exclusion moves into an EF anti-join (either direction); rows are ordered accepted-first, newest-first and capped at MaxFriends (was unbounded). Deactivated counterparties still drop via the #324 User query filter on the display-name resolve. - GetCheers: adds a 90-day lookback and a 200-row cap, newest-first, with blocked senders/recipients excluded in the query. Behavior-preserving for realistic data (accepted friends are hard-capped at 500; the cheers list is a bounded feed) and the response DTOs are unchanged, so there is no consumer contract change. Reader query composition is exposed as internal statics and unit-tested via LINQ-to-objects at the filtering/ordering/pagination boundaries. Refs thomasluizon/orbit-ui-mobile#243 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Review Summary
Recommendation: APPROVE
| Severity | Count |
|---|---|
| Critical (incl. breaks old mobile clients) | 0 |
| High | 0 |
| Medium | 2 |
| Low / Info | 0 (not posted per signal gate) |
Summary
This PR replaces in-memory filtering/capping in GetFriendsQuery and GetCheersQuery with a new ISocialGraphReader (mirroring the existing IFriendFeedReader pattern) that pushes the blocked-user anti-join, ordering, a 90-day lookback (cheers), and row caps into the EF query. Response DTOs (FriendsResponse, CheersPage) are unchanged — no contract drift. A dedicated security review verified bidirectional block exclusion is correct in both query builders, SocialAccessGuard + [Authorize] still gate both handlers, UserId still comes only from the JWT claim, and TimeProvider injection correctly avoids raw DateTime.UtcNow in Orbit.Application. Test coverage is strong at both the Infrastructure (query-composition via LINQ-to-objects) and Application (handler wiring) layers.
Findings
[Medium] Friend/pending-request row cap can silently starve pending requests near MaxFriends
src/Orbit.Infrastructure/Persistence/SocialGraphReader.cs:192-206(BuildVisibleFriendships),src/Orbit.Application/Social/Commands/SendFriendRequestCommand.cs:40Take(limit)caps the combined accepted+pending friendship rows atMaxFriends=500, ordered accepted-first — butSendFriendRequestCommand's cap check (CountAcceptedFriendsAsync) only counts accepted friendships against that same 500, so pending rows are effectively unbounded by that check. A user near 500 accepted friends with outstanding requests can have incoming/outgoing requests silently dropped from the response with no error, whereas the pre-PR code always returned every pending row (unbounded).- Fix: cap accepted and pending partitions independently rather than sharing one
Take(limit).
[Medium] Cheer.RecipientId lacks the composite index this PR's DB-pushdown needs
src/Orbit.Infrastructure/Persistence/OrbitDbContext.cs:531-532(ConfigureCheerEntity), exercised bySocialGraphReader.cs:208-231- The existing config has
(SenderId, CreatedAtUtc)as a composite index but only a single-column index onRecipientId. The new "received" query filters byRecipientId+CreatedAtUtc >= sinceand orders byCreatedAtUtc DESC— exactly the shape the sender-side composite was built for, but the received side (likely the more common direction) is missing its mirror, undermining the perf goal of this exact PR. - Fix: add
entity.HasIndex(c => new { c.RecipientId, c.CreatedAtUtc })with a migration.
Subagents
| Agent | Verdict |
|---|---|
| security-reviewer | PASS |
| contract-aligner | N/A — no DTO/route/shared-type changed |
Validation
Skipped per orchestration instructions — Build / Unit Tests / SonarCloud run as separate required CI checks on this PR.
What's good
Clean port/adapter split following the IFriendFeedReader precedent; bidirectional block-exclusion is correct; query builders are pragmatically unit-testable via LINQ-to-objects given the repo's no-integration-suite constraint; TimeProvider correctly avoids raw DateTime.UtcNow in Application; test updates precisely assert the new reader-call parameters (cap values, lookback window).
Both Medium findings are real but neither is Critical/High — the friend-cap interaction is a rare edge case (near 500 connections) and the index gap is performance-only with no correctness impact. Safe to land with these as tracked follow-ups.
There was a problem hiding this comment.
PR Review: #344 — perf(api): push social query filtering + pagination into the DB
Recommendation: APPROVE
Summary
The current head commit (1c7edc6) is identical, on this PR's actual file surface, to the commit already reviewed and approved previously — the merge only pulled in unrelated main work (auth-session/rate-limiting changes) that doesn't touch any of the 10 files this PR modifies (GetFriendsQuery.cs, GetCheersQuery.cs, ISocialGraphReader.cs, SocialGraphReader.cs, AppConstants.cs, ServiceCollectionExtensions.cs, and 4 test files). There is no new diff to review since the prior approval.
- 0 Critical, 0 High, 2 Medium findings — none block merge.
- Security: PASS — bidirectional block-exclusion is correct in both query builders, SocialAccessGuard/[Authorize] intact, UserId sourced only from the JWT claim, and TimeProvider is used correctly (no raw DateTime.UtcNow in Orbit.Application).
- Cross-repo contract: N/A — no DTO/route/packages/shared type changed in this PR; response DTOs (FriendsResponse, CheersPage) are unchanged, so there's no backward-compat risk to verify against orbit-ui-mobile (which isn't checked out in this job).
Medium findings (non-blocking, tracked as follow-up)
- src/Orbit.Infrastructure/Persistence/SocialGraphReader.cs:192-206 — the combined Take(limit) in BuildVisibleFriendships caps accepted+pending rows together at MaxFriends=500, but SendFriendRequestCommand's cap check only counts accepted friendships against that same limit. Pending requests can be silently starved near the cap (pre-PR behavior always returned all pending rows).
- src/Orbit.Infrastructure/Persistence/OrbitDbContext.cs:531-532 — Cheer.RecipientId has only a single-column index; the new "received" cheers query filters on RecipientId + CreatedAtUtc >= since and orders by CreatedAtUtc DESC — the same access shape the existing (SenderId, CreatedAtUtc) composite index was built for. A mirrored (RecipientId, CreatedAtUtc) composite would avoid a filter+sort scan on the single-column index as cheer volume grows.
Neither issue is a correctness bug; both are reasonable perf/capacity follow-ups.
|



What
GetFriendsQueryandGetCheersQuerymaterialized a user's entire friendship / cheer history and then filtered blocked users + capped the result in memory. This moves the filtering, ordering, lookback window, and row cap into the EF query via a newISocialGraphReader(Infrastructure), following the existingIFriendFeedReaderpattern.GetFriends
BlockedUser(either direction) instead of loading all blocks and filtering in memory.MaxFriends(was unbounded). Accepted-first ordering guarantees the cap never drops a real friend before a pending request.User!IsDeactivatedglobal filter on the display-name resolve (unchanged).GetCheers
CheersLookbackDays) and a 200-row cap (MaxCheersReturned), newest-first, with blocked senders/recipients excluded in the query.nowcomes from the injectedTimeProvider(keeps thecsharp-tzrule happy and makes the window testable).Behavior preservation
Behavior-preserving for realistic data: accepted friends are hard-capped at 500 (
SendFriendRequestCommand), and the cheers list is a bounded feed. The response DTOs (FriendsResponse,CheersPage) are unchanged, so there is no consumer contract change (nopackages/shared/ mobile / web edit needed).Tests
SocialGraphReaderTests(Infrastructure) exercises the reader's query composition via LINQ-to-objects at the boundaries: blocked exclusion in both directions, ignoring rows not involving the user, accepted-first/newest-first ordering, the row cap, direction routing, and the lookback boundary (row exactly atsincekept, one tick older dropped).GetFriendsQueryTests/GetCheersQueryTests(Application) cover partitioning, display-field mapping, theMaxFriendscap, the lookback window + page cap passed to the reader, unknown-sender mapping, and the opted-out short-circuit.Refs thomasluizon/orbit-ui-mobile#243