Skip to content

Conversation

@seapagan
Copy link
Owner

@seapagan seapagan commented Jan 14, 2026

Summary

  • Created invalidate_user_related_caches() helper that uses asyncio.gather() to invalidate caches in parallel
  • Updated all 5 user mutation endpoints (make_admin, ban, unban, edit, delete) to use the new helper
  • Replaces sequential cache invalidation with concurrent execution, reducing total invalidation time

Refers to SECURITY-REVIEW.md issue #33

Summary by CodeRabbit

  • Performance

    • User-related cache invalidation is now combined and executed in parallel, improving responsiveness after user updates.
  • Refactor

    • User management actions (promote, ban, unban, edit, delete) now use the consolidated cache invalidation flow.
  • Documentation

    • Caching guide updated to prefer namespace constants, expanded examples, and a recommended combined-user-cache invalidation pattern.

✏️ Tip: You can customize this high-level summary in your review settings.

Replace sequential cache invalidation calls with parallel execution
using asyncio.gather(). New invalidate_user_related_caches() helper
invalidates user-specific and users-list caches concurrently,
reducing total invalidation time.

Refers to SECURITY-REVIEW.md issue #33
@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

Added an asynchronous helper invalidate_user_related_caches(user_id) that concurrently invalidates per-user and users-list caches; updated all five user mutation endpoints to call this helper instead of calling individual invalidation functions.

Changes

Cohort / File(s) Summary
Cache invalidation consolidation
app/cache/invalidation.py, app/cache/__init__.py
New async helper invalidate_user_related_caches(user_id) implemented using asyncio.gather; exported via app.cache.__init__.
User endpoint refactoring
app/resources/user.py
Replaced separate invalidation calls with a single call to invalidate_user_related_caches(user_id) in make_admin, ban_user, unban_user, edit_user, and delete_user.
Documentation
docs/usage/caching.md
Examples updated to prefer CacheNamespaces constants and added a “Combined User Cache Invalidation (Recommended)” pattern referencing the new helper.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API as UserEndpoint
    participant Inval as InvalidationHelper
    participant Cache as CacheStore

    Client->>API: perform user mutation (e.g. ban, edit)
    API->>Inval: call invalidate_user_related_caches(user_id)
    Inval->>Cache: invalidate user-specific cache (async)
    Inval->>Cache: invalidate users-list cache (async)
    Note right of Inval: both invalidations run concurrently via asyncio.gather
    Cache-->>Inval: acknowledgements
    Inval-->>API: completion
    API-->>Client: mutation response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

enhancement

Poem

🐰 I hop and hum, with caches twined,

Two clears at once — no lag assigned.
Async paws that dance and play,
Parallel hops to clear the way.
A cheery rabbit, cache-joy aligned 🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main objective of the pull request: introducing parallel cache invalidation using asyncio.gather to improve performance.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.



📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 808edbc and 9e92b88.

📒 Files selected for processing (1)
  • docs/usage/caching.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: test (3.13)
  • GitHub Check: test (3.14)
  • GitHub Check: test (3.10)
  • GitHub Check: test (3.11)
  • GitHub Check: test (3.12)
🔇 Additional comments (5)
docs/usage/caching.md (5)

213-228: Good documentation of constant usage pattern.

The import statement, usage example, and tip box clearly demonstrate the recommended practice of using CacheNamespaces constants. This helps prevent typos and improves code maintainability.


258-267: Consistent reinforcement of namespace constants.

The custom key builders section correctly demonstrates using CacheNamespaces.USER_ME with the decorator parameters, maintaining consistency with the basic usage examples.


313-329: Good documentation of alternative invalidation approach.

Documenting both the combined helper and individual invalidation functions gives users flexibility to choose the appropriate approach for their use case. This is helpful for scenarios requiring fine-grained cache control.


122-134: CacheNamespaces constants verified and accurate.

All constants referenced in lines 122-134 exist at app.cache.constants with the correct values. The documentation accurately represents the implementation.


293-312: No issues found. All technical claims in the documentation are accurate:

  • invalidate_user_related_caches() function exists at app/cache/invalidation.py:120 and is correctly exported from app.cache
  • The function uses asyncio.gather() for concurrent execution as documented
  • All three cache namespaces listed are correctly cleared: user:{user_id}, users:{user_id}, and users:list

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codacy-production
Copy link

codacy-production bot commented Jan 14, 2026

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%) 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (f66d715) 2442 2442 100.00%
Head commit (9e92b88) 2440 (-2) 2440 (-2) 100.00% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#815) 8 8 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

…idation

- Add CacheNamespaces constants reference to avoid hardcoded strings
- Update examples to show invalidate_user_related_caches() for parallel
  invalidation instead of sequential calls
- Add tip recommending use of namespace constants
- Update all code examples to use best practices

Relates to PR #814 (namespace constants) and PR #815 (parallel invalidation)
@seapagan seapagan self-assigned this Jan 14, 2026
@seapagan seapagan added the enhancement New feature or request label Jan 14, 2026
@seapagan seapagan merged commit ec76dff into main Jan 14, 2026
18 checks passed
@seapagan seapagan deleted the fix/cache-invalidation-performance branch January 14, 2026 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants