chore: Implement short-lived redis cache for slots#22787
chore: Implement short-lived redis cache for slots#22787ThyMinimalDev merged 29 commits intomainfrom
Conversation
WalkthroughThis change introduces Redis caching for available slots within the application. It adds a Redis service implementation with connection status tracking, connection lifecycle event logging, error handling, and common Redis operations such as get, set (with optional TTL), del, expire, lrange, and lpush. A no-operation Redis service stub is added as a fallback when Redis environment variables are not configured. Dependency injection tokens and modules are updated to support the new Redis service, including conditional binding to either the real or noop Redis service. The Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
…com/cal.com into chore/shortlived-slots-cache
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/features/redis/di/redisModule.ts (1)
10-16: LGTM! Graceful fallback pattern is well implemented.The factory function correctly provides graceful degradation from RedisService to NoopRedisService when Redis is unavailable. The singleton binding ensures consistent behavior across the application.
Consider adding optional error logging to aid debugging in production environments:
redisModule.bind(DI_TOKENS.REDIS_CLIENT).toFactory(() => { try { return new RedisService(); - } catch (error) {} + } catch (error) { + // Optional: Log for debugging in production + // console.warn("Redis unavailable, falling back to noop service:", error); + } // or fall back to a noop service if Redis is not available return new NoopRedisService(); }, "singleton");packages/trpc/server/routers/viewer/slots/util.ts (1)
139-139: Consider making TTL configurableThe hardcoded 2-second TTL might not be optimal for all scenarios. Consider making this configurable through environment variables or service configuration.
- redisClient.set(cacheKey, result, { ttl: 2000 }); // Cache for 2 seconds + const ttl = process.env.SLOTS_CACHE_TTL_MS ? parseInt(process.env.SLOTS_CACHE_TTL_MS) : 2000; + redisClient.set(cacheKey, result, { ttl }); // Cache with configurable TTL
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
apps/api/v2/src/lib/modules/available-slots.module.ts(2 hunks)apps/api/v2/src/lib/services/available-slots.service.ts(3 hunks)apps/api/v2/src/modules/redis/redis.service.ts(2 hunks)apps/web/package.json(1 hunks)packages/features/redis/IRedisService.d.ts(1 hunks)packages/features/redis/NoopRedisService.ts(1 hunks)packages/features/redis/RedisService.ts(2 hunks)packages/features/redis/di/redisModule.ts(1 hunks)packages/lib/di/containers/available-slots.ts(2 hunks)packages/lib/di/modules/available-slots.ts(1 hunks)packages/lib/di/tokens.ts(1 hunks)packages/trpc/server/routers/viewer/slots/util.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/lib/modules/available-slots.module.tspackages/lib/di/containers/available-slots.tspackages/lib/di/tokens.tspackages/lib/di/modules/available-slots.tsapps/api/v2/src/lib/services/available-slots.service.tspackages/features/redis/RedisService.tspackages/features/redis/di/redisModule.tspackages/features/redis/IRedisService.d.tspackages/trpc/server/routers/viewer/slots/util.tspackages/features/redis/NoopRedisService.tsapps/api/v2/src/modules/redis/redis.service.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/lib/modules/available-slots.module.tspackages/lib/di/containers/available-slots.tspackages/lib/di/tokens.tspackages/lib/di/modules/available-slots.tsapps/api/v2/src/lib/services/available-slots.service.tspackages/features/redis/RedisService.tspackages/features/redis/di/redisModule.tspackages/features/redis/IRedisService.d.tspackages/trpc/server/routers/viewer/slots/util.tspackages/features/redis/NoopRedisService.tsapps/api/v2/src/modules/redis/redis.service.ts
**/*.{service,repository}.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/lib/services/available-slots.service.tsapps/api/v2/src/modules/redis/redis.service.ts
**/*Service.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/features/redis/RedisService.tspackages/features/redis/NoopRedisService.ts
🧠 Learnings (4)
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/lib/di/modules/available-slots.ts
📚 Learning: applies to **/*repository.ts : repository files must include `repository` suffix, prefix with techno...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class
Applied to files:
apps/api/v2/src/lib/services/available-slots.service.ts
📚 Learning: the office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is spec...
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Applied to files:
packages/trpc/server/routers/viewer/slots/util.ts
📚 Learning: in the insightsbookingservice (packages/lib/server/service/insightsbooking.ts), the constructor stor...
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.
Applied to files:
packages/trpc/server/routers/viewer/slots/util.ts
🧬 Code Graph Analysis (5)
apps/api/v2/src/lib/modules/available-slots.module.ts (1)
packages/features/redis/RedisService.ts (1)
RedisService(5-46)
packages/lib/di/containers/available-slots.ts (2)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-30)packages/features/redis/di/redisModule.ts (1)
redisModule(18-18)
packages/lib/di/modules/available-slots.ts (1)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-30)
apps/api/v2/src/lib/services/available-slots.service.ts (1)
packages/features/redis/RedisService.ts (1)
RedisService(5-46)
packages/features/redis/NoopRedisService.ts (1)
packages/features/redis/IRedisService.d.ts (1)
IRedisService(1-13)
⏰ 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). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
🔇 Additional comments (23)
apps/web/package.json (1)
82-82: No breaking changes or security advisories detected for @upstash/redis v1.21.0 → v1.35.2
- Checked GitHub release notes from v1.21.0 through v1.35.2 for “BREAKING” changes: none found.
- Queried NPM security advisories for @upstash/redis: no vulnerabilities reported.
This dependency bump can be merged as-is.
packages/lib/di/tokens.ts (1)
5-5: LGTM!The new
REDIS_CLIENTtoken follows the established pattern and naming convention. The addition is consistent with the existing dependency injection structure.apps/api/v2/src/lib/modules/available-slots.module.ts (2)
12-12: LGTM!Proper import of
RedisServicefollowing the established module import pattern.
26-26: LGTM!Correctly adds
RedisServiceto the providers array, making it available for dependency injection within the module.packages/lib/di/containers/available-slots.ts (2)
3-3: LGTM!Proper import of
redisModulefollowing the established import pattern for DI modules.
20-20: LGTM!Correctly loads the Redis module into the container using the appropriate DI token. The placement follows the logical sequence with other module loadings.
packages/lib/di/modules/available-slots.ts (1)
18-18: LGTM!Properly adds the Redis client dependency binding following the established pattern. The
satisfiesconstraint ensures type safety with the service interface.apps/api/v2/src/lib/services/available-slots.service.ts (2)
9-9: LGTM! Redis service import is correctly added.The import follows the established pattern and correctly references the RedisService from the NestJS modules directory.
25-27: LGTM! Redis service dependency injection is properly implemented.The constructor correctly accepts the RedisService parameter and passes it to the base class as
redisClient, enabling Redis caching functionality for available slots.Also applies to: 37-37
packages/features/redis/IRedisService.d.ts (1)
4-4: LGTM! TTL support added to the set method interface.The optional
optsparameter withttlproperty enables time-to-live functionality while maintaining backward compatibility. This aligns with the PR objective of implementing short-lived Redis cache.packages/features/redis/RedisService.ts (2)
9-12: LGTM! Request timeout configuration improves reliability.Adding a 2-second timeout prevents hanging Redis requests and improves service reliability. The timeout value is appropriate for Redis operations.
22-32: LGTM! TTL implementation correctly uses Redis PX option.The set method properly implements TTL support using the
pxoption for millisecond precision. The conditional logic ensures TTL is only applied when provided, maintaining backward compatibility.packages/features/redis/NoopRedisService.ts (1)
7-32: LGTM! Noop implementation provides safe fallback behavior.The NoopRedisService correctly implements the IRedisService interface with appropriate default return values. The unused parameter prefixes and clear documentation make the intent obvious. This provides a safe fallback when Redis is unavailable without breaking dependent code.
packages/trpc/server/routers/viewer/slots/util.ts (3)
11-11: LGTM - Clean import additionThe import of
IRedisServicetype is correctly placed and follows the existing import pattern.
95-105: Interface reordering and Redis client addition looks goodThe addition of
redisClient: IRedisServiceto the interface is appropriate for dependency injection. The reordering ofcheckBookingLimitsServiceaboveredisClientmaintains consistency.
925-928: Clean integration of caching with reportingThe wrapping of
_getAvailableSlotswith bothwithSlotsCacheandwithReportingfollows a good composition pattern. The order ensures caching happens before reporting, which is appropriate.apps/api/v2/src/modules/redis/redis.service.ts (7)
10-10: Good addition of connection status trackingAdding
isReadyto track connection status is a good practice for preventing operations on disconnected clients.
18-35: Comprehensive connection event handlingThe event handlers provide good visibility into Redis connection state and properly update the
isReadyflag. The logging levels are appropriate for each event type.
46-67: Robust get method with good error handlingThe implementation properly checks connection status, handles errors gracefully, and attempts JSON parsing with fallback to raw data. The type safety with generics is well implemented.
69-79: Clean del method implementationThe delete method follows the same pattern as get with proper error handling and connection status checking.
81-99: Well-implemented set method with TTL supportThe set method properly handles both object and primitive types, supports optional TTL using Redis PX (milliseconds), and includes comprehensive error handling.
101-111: Clean expire method implementationThe expire method follows the established pattern with proper error handling and type safety.
113-139: List operations implemented correctlyBoth
lrangeandlpushmethods handle JSON serialization/deserialization appropriately and follow the same error handling pattern as other methods. The automatic JSON parsing inlrangeassumes all list items are JSON, which is reasonable for this use case.
E2E results are ready! |
lol |
btwitsPratyush
left a comment
btwitsPratyush
left a comment
There was a problem hiding this comment.
changes look good overall. good to merge once tested.
| EventType, | ||
| GetAvailabilityUser, | ||
| UserAvailabilityService, | ||
| IUserAvailabilityService, |
There was a problem hiding this comment.
needs to be UserAvailabilityService
apps/api/v2/package.json
Outdated
| "@calcom/platform-constants": "*", | ||
| "@calcom/platform-enums": "*", | ||
| "@calcom/platform-libraries": "npm:@calcom/platform-libraries@0.0.283", | ||
| "@calcom/platform-libraries": "npm:@calcom/platform-libraries@0.0.284", |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
4 participants
What does this PR do?