-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add project permission cache #4577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR introduces comprehensive project permission caching to reduce database load on frequently accessed permission endpoints. The implementation adds a 5-minute cache layer for user and identity project permissions using pattern-based cache keys. Cache invalidation is systematically implemented across all services that modify permissions, roles, or memberships.
Key Changes
- Permission caching: Added
getProjectPermission()
cache with 5-minute TTL - Cache invalidation: Implemented across 9+ service files when permissions change
- Pattern-based keys: Uses structured cache keys for efficient bulk invalidation
- Service integration: Added
keyStore
dependency to permission service
Implementation Quality
The cache invalidation logic is thorough and correctly placed after all permission-related operations. The code uses Promise.allSettled()
for cache invalidation to prevent failures from blocking operations. Cache keys are well-structured with actor-specific patterns for efficient bulk deletion.
Confidence Score: 4/5
- This PR is largely safe to merge with good caching implementation and thorough invalidation logic
- Score reflects the solid cache invalidation pattern and comprehensive coverage, with minor concerns about service token cache bypass and error handling
- Pay attention to permission-service.ts for the service token cache bypass behavior and silent error handling in cache operations
Important Files Changed
File Analysis
Filename | Score | Overview |
---|---|---|
backend/src/ee/services/permission/permission-service.ts | 4/5 | Adds comprehensive project permission caching with 5-minute TTL, but service tokens bypass cache and error handling needs improvement |
backend/src/keystore/keystore.ts | 5/5 | Defines cache key patterns and TTL for project permissions - clean implementation with proper separation of concerns |
backend/src/ee/services/group/group-service.ts | 5/5 | Properly invalidates both user and project permission caches when group memberships/roles change |
backend/src/services/project-membership/project-membership-service.ts | 5/5 | Cache invalidation correctly implemented for project membership operations affecting user and project permission caches |
Sequence Diagram
sequenceDiagram
participant Client
participant PermissionService
participant KeyStore
participant Database
Note over Client,Database: Permission Check with Cache
Client->>PermissionService: getProjectPermission()
PermissionService->>PermissionService: Generate cache key
PermissionService->>KeyStore: getItem(cacheKey)
alt Cache Hit
KeyStore-->>PermissionService: Cached permission data
PermissionService->>PermissionService: Reconstruct permission object
PermissionService-->>Client: Permission result
else Cache Miss
PermissionService->>Database: Query user/identity permissions
Database-->>PermissionService: Permission data
PermissionService->>PermissionService: Build permission rules
PermissionService->>KeyStore: setItemWithExpiry(cacheKey, 5min, data)
PermissionService-->>Client: Permission result
end
Note over Client,Database: Permission Update with Cache Invalidation
Client->>PermissionService: updateProjectMembership()
PermissionService->>Database: Update membership
Database-->>PermissionService: Updated membership
PermissionService->>PermissionService: invalidateProjectPermissionCache()
PermissionService->>KeyStore: deleteItems(project pattern)
PermissionService->>PermissionService: invalidateUserProjectPermissionCache()
PermissionService->>KeyStore: deleteItems(user pattern)
PermissionService-->>Client: Update complete
12 files reviewed, 2 comments
Description 📣
Add a cache level on permissions endpoints to reduce the DB impact these queries have on many of the functions/endpoints. Should invalidate correctly when permissions change
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets