feat(flags): add CachedFeaturesRepository with Redis caching#26123
feat(flags): add CachedFeaturesRepository with Redis caching#26123eunjae-lee wants to merge 27 commits intomainfrom
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
bc58bb0 to
d075b06
Compare
57901a7 to
7446133
Compare
d075b06 to
496dc13
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
7446133 to
ade2ecb
Compare
ade2ecb to
4968e81
Compare
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…+ featureIds) - getUserFeatureStates now uses userId + featureIds as composite cache key - getTeamsFeatureStates now uses teamId + featureIds as composite cache key per team - Removed partial cache merging logic for simpler full hit/miss pattern - Single feature checks (checkIfUserHasFeatureNonHierarchical, checkIfTeamHasFeature) delegate to repository - Feature state mutations no longer invalidate cache (rely on TTL) - Added normalizeFeatureIds to ensure consistent cache key generation - Updated tests to reflect new caching behavior Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Extract shared per-team cache resolution and align cache types with partial feature maps. Generated with [opencode](https://opencode.ai) Co-Authored-By: opencode <noreply@opencode.ai>
- Add schema.safeParse validation before storing values in cache - Throw error with cache key in message if validation fails - Add 6 tests for invalid write scenarios Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
424f2d4 to
0019709
Compare
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…duleLoader pattern - Add Redis moduleLoader in packages/features/redis/di/redisModule.ts - Add CachedFeaturesRepository tokens to packages/features/flags/di/tokens.ts - Create moduleLoader for CachedFeaturesRepository with depsMap (featuresRepository + redisService) - Create container function getCachedFeaturesRepository - Update CachedFeaturesRepository constructor to accept deps object for DI compatibility - Update tests to use new constructor signature Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…tory - Update FeatureOptInService module to use CachedFeaturesRepository instead of raw FeaturesRepository - Update FeatureOptInService class to use IFeaturesRepository interface for flexibility Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/di/modules/FeatureOptInService.ts">
<violation number="1" location="packages/features/di/modules/FeatureOptInService.ts:5">
P1: Using CachedFeaturesRepository for FeatureOptInService creates read-after-write consistency issues. The service performs mutations (setUserFeatureState, setTeamFeatureState) that don't invalidate caches, then reads (getUserFeatureStates, getTeamsFeatureStates) that return stale cached data for up to 24 hours. Users will enable features but see them as disabled until cache expires.
Consider keeping FeatureOptInService on the direct FeaturesRepository, or implement cache invalidation for composite keys when mutations occur.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. |
Reverts FeatureOptInService to use FeaturesRepository instead of CachedFeaturesRepository to avoid read-after-write consistency issues. The service performs mutations (setUserFeatureState, setTeamFeatureState) that don't invalidate caches, which would cause users to see stale data for up to 24 hours after enabling features. CachedFeaturesRepository remains available for read-only use cases. Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/di/modules/FeatureOptInService.ts">
<violation number="1">
P0: FeatureOptInService is wired to use FeaturesRepository instead of CachedFeaturesRepository, contradicting the PR's stated goal. This change bypasses the Redis caching layer entirely, defeating the purpose of this PR.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. |
volnei
left a comment
There was a problem hiding this comment.
@eunjae-lee since we're touching here, I think we need a better refactor on this...
- separate features.repository by global/user/team
- create a service to handle instead of handle it always from repositories
- create a wrapper that enable cache (memoize) as I think it should be responsibility of a service
- create a endpoint to enable faster clear cache (with a secret)
- create a CachedFeatureFlagService/Repository that fallback to database when redis is not available
- No more provider being accessed directly
|
Closing to start fresh with separate repositories as discussed in the review feedback. |
…lags This PR introduces 6 separate repository classes for feature flags: Prisma Repositories (Database Layer): - PrismaFeatureRepository: Global Feature table operations - PrismaTeamFeatureRepository: TeamFeatures table operations - PrismaUserFeatureRepository: UserFeatures table operations Redis Repositories (Cache Layer): - RedisFeatureRepository: Redis caching for global features - RedisTeamFeatureRepository: Redis caching for team features - RedisUserFeatureRepository: Redis caching for user features This separation follows the feedback from PR #26123 to: 1. Separate features.repository by global/user/team 2. Enable a service layer to orchestrate these repositories 3. Support caching wrappers that can be added later Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
- Add moduleLoader export directly to packages/features/redis/di/redisModule.ts following the pattern from PR #26123 - Remove packages/features/di/modules/Redis.ts (was creating unnecessary indirection) - Update Redis repository modules to import from redisModule.ts directly Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

What does this PR do?
Adds a Redis caching layer for
FeaturesRepositoryusing the Proxy Pattern, similar toBillingServiceCachingProxy. TheCachedFeaturesRepositorywraps the existing repository and transparently caches read operations with zero code changes to consumers.Key Features
userId/teamId + featureIds(e.g.,features:userFeatureStates:123:emails,insights)featureIdsorderUpdates since last revision
getCachedFeaturesRepository()for read-only use casesFeatureOptInServiceclass to useIFeaturesRepositoryinterface for flexibilityCaching Strategy
Cached operations (24-hour TTL):
getUserFeatureStates- Composite key:userId + featureIdsgetTeamsFeatureStates- Composite key per team:teamId + featureIdsgetAllFeatures,checkIfFeatureIsEnabledGlobally,getTeamsWithFeatureEnabledgetUserAutoOptIn,getTeamsAutoOptIn- Exact invalidation on mutationNot cached (always delegate to repository):
checkIfUserHasFeature,checkIfUserHasFeatureNonHierarchical- Hierarchical resolution requires cross-entity logiccheckIfTeamHasFeature- Single feature lookups inefficient with composite keysCache invalidation:
setUserAutoOptIn/setTeamAutoOptInArchitecture
Important Notes
Eventual consistency: Feature state mutations don't invalidate composite key caches. Data may be stale for up to 24 hours after
setUserFeatureState/setTeamFeatureState. This tradeoff simplifies cache invalidation logic.Not wired to FeatureOptInService: Due to read-after-write consistency concerns, FeatureOptInService continues to use the direct FeaturesRepository. CachedFeaturesRepository is available for read-heavy use cases that can tolerate eventual consistency.
Single feature checks not cached:
checkIfUserHasFeature*andcheckIfTeamHasFeaturealways hit the database. If these are performance-critical, we can add per-feature caching in a follow-up.Files Changed
Core implementation:
packages/features/flags/cached-features.repository.ts- Proxy implementation with DI-compatible constructorpackages/features/flags/features-cache-keys.ts- Cache key definitions + Zod schemasDI wiring:
packages/features/di/modules/CachedFeaturesRepository.ts- moduleLoader with depsMappackages/features/di/containers/CachedFeaturesRepository.ts- Container functionpackages/features/flags/di/tokens.ts- DI tokenspackages/features/redis/di/redisModule.ts- Redis moduleLoader exportService updates:
packages/features/feature-opt-in/services/FeatureOptInService.ts- Uses IFeaturesRepository interfaceTests:
packages/features/flags/__tests__/cached-features.repository.test.ts- 39 unit testspackages/features/flags/__tests__/spy-features.repository.ts- Mock repositorypackages/features/flags/__tests__/mock-redis.service.ts- In-memory Redis mockMandatory Tasks (DO NOT REMOVE)
How should this be tested?
Environment: Requires
UPSTASH_REDIS_REST_URLandUPSTASH_REDIS_REST_TOKENenvironment variables for Redis caching to be active (falls back to NoopRedisService otherwise)Unit tests: Run
yarn test packages/features/flags/__tests__/- 39 tests covering all scenariosType checks: Run
yarn type-check:ci --forceReview Checklist
safeParse()on every cache read/write is acceptableChecklist
Link to Devin run: https://app.devin.ai/sessions/a6e8ab4bdb8b43cb96fad0904ca278b6
Requested by: eunjae@cal.com (@eunjae-lee)