Update redis fetch from db #56
Conversation
WalkthroughThe changes include updating the visibility of the no-argument constructor in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JwtAuthenticationUtil
participant Database
participant Redis
Client->>JwtAuthenticationUtil: fetchUserFromDB(userId)
JwtAuthenticationUtil->>Database: Query user by userId
Database-->>JwtAuthenticationUtil: Return full user object
JwtAuthenticationUtil->>JwtAuthenticationUtil: Create minimal user object (userID, userName)
JwtAuthenticationUtil->>Redis: Cache minimal user object (30 min expiry)
JwtAuthenticationUtil-->>Client: Return full user object
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/iemr/helpline104/data/users/M_User.java (1)
98-99: Consider the security implications of making the entity constructor public.Making the no-argument constructor public enables external instantiation of
M_Userobjects, which could lead to incomplete entity states if not used carefully. While this change is necessary for the caching optimization inJwtAuthenticationUtil, ensure that this constructor is only used for legitimate purposes like creating minimal cache objects.Consider adding documentation to clarify the intended usage:
+ /** + * Default constructor for creating minimal user objects for caching purposes. + * Use with caution - ensure proper field initialization. + */ public M_User() { }src/main/java/com/iemr/helpline104/utils/JwtAuthenticationUtil.java (1)
120-120: Consider configurable cache expiration time.The 30-minute cache expiration is hardcoded. Consider making this configurable through application properties to allow for different environments or requirements.
Create a configuration property:
+@Value("${redis.user.cache.expiration.minutes:30}") +private int userCacheExpirationMinutes;Then use it in the cache operation:
-redisTemplate.opsForValue().set(redisKey, userHash, 30, TimeUnit.MINUTES); +redisTemplate.opsForValue().set(redisKey, userHash, userCacheExpirationMinutes, TimeUnit.MINUTES);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/iemr/helpline104/data/users/M_User.java(1 hunks)src/main/java/com/iemr/helpline104/utils/JwtAuthenticationUtil.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
🔇 Additional comments (2)
src/main/java/com/iemr/helpline104/utils/JwtAuthenticationUtil.java (2)
115-117: Good optimization for memory efficiency.Creating a minimal user object with only essential fields (
userIDanduserName) for caching is an excellent optimization that reduces memory usage and serialization overhead while maintaining the functionality needed for JWT validation.
107-131:Details
✅ Verification successful
Verify that minimal user caching doesn't break dependent functionality.
The optimization to cache only
userIDanduserNamereduces memory usage, which is good. However, this could be a breaking change if other parts of the codebase expect the cached user object to contain all user properties.Run the following script to verify that no other code depends on accessing additional user properties from the cached user object:
🏁 Script executed:
#!/bin/bash # Description: Search for code that retrieves users from cache and accesses properties other than userID and userName # Search for getUserFromCache usage and check what properties are accessed ast-grep --pattern $'getUserFromCache($$$)' # Search for Redis user key access patterns rg -A 10 "user_.*redisTemplate|redisTemplate.*user_" # Search for M_User property access that might depend on cached data rg -A 5 "\.get[A-Z][a-zA-Z]*\(\)" --type java | grep -v "getUserID\|getUserName"Length of output: 115028
No breaking changes detected for minimal user caching
Searches for
getUserFromCachecall sites and all otherM_Userproperty getters returned onlygetUserID()andgetUserName(). There are no consumers expecting additional fields from the cached user object, so caching only those two properties won’t break existing functionality.
📋 Description
JIRA ID:
Update redis fetch from db
✅ Type of Change
Summary by CodeRabbit
No user-facing features or interface changes were introduced.