3.3.0 #227
3.3.0 #227
Conversation
Beneficiary consent
Firebase setup
Beneficiaries consent
# Conflicts: # pom.xml # src/main/environment/common_ci.properties # src/main/environment/common_dev.properties # src/main/environment/common_test.properties # src/main/environment/common_uat.properties # src/main/java/com/iemr/common/controller/grievance/GrievanceController.java # src/main/java/com/iemr/common/data/grievance/GrievanceCallRequest.java # src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java # src/main/java/com/iemr/common/repository/callhandling/IEMRCalltypeRepositoryImplCustom.java # src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java # src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java # src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java # src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java # src/main/java/com/iemr/common/utils/http/HTTPRequestInterceptor.java
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the """ WalkthroughThis update introduces Firebase Cloud Messaging integration and a new beneficiary consent flow. It adds dependencies and configuration for Firebase, implements OTP handling logic for beneficiary consent via new controllers and services, and updates data models. Several new classes and endpoints are added, with some minor adjustments to existing logging and model fields. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BeneficiaryConsentController
participant BeneficiaryOTPHandler
participant SMS Gateway
Client->>BeneficiaryConsentController: POST /sendConsent (beneficiary data)
BeneficiaryConsentController->>BeneficiaryOTPHandler: sendOTP(request)
BeneficiaryOTPHandler->>SMS Gateway: Send OTP SMS
SMS Gateway-->>BeneficiaryOTPHandler: SMS sent response
BeneficiaryOTPHandler-->>BeneficiaryConsentController: OTP send result
BeneficiaryConsentController-->>Client: Success/Error response
Client->>BeneficiaryConsentController: POST /validateConsent (mobile, OTP)
BeneficiaryConsentController->>BeneficiaryOTPHandler: validateOTP(request)
BeneficiaryOTPHandler-->>BeneficiaryConsentController: Validation result
BeneficiaryConsentController-->>Client: Success/Error response
sequenceDiagram
participant Client
participant FirebaseNotificationController
participant FirebaseNotificationService
participant FirebaseMessaging (SDK)
Client->>FirebaseNotificationController: POST /firebaseNotification/sendNotification (NotificationMessage)
FirebaseNotificationController->>FirebaseNotificationService: sendNotification(notificationMessage)
FirebaseNotificationService->>FirebaseMessaging: Send notification
FirebaseMessaging-->>FirebaseNotificationService: Response
FirebaseNotificationService-->>FirebaseNotificationController: Result string
FirebaseNotificationController-->>Client: Response
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/main/java/com/iemr/common/controller/beneficiaryConsent/BeneficiaryConsentController.java
Fixed
Show fixed
Hide fixed
| @Operation(summary = "Resend Consent") | ||
| @RequestMapping(value = "/resendConsent", method = RequestMethod.POST, consumes = MediaType.APPLICATION_JSON, produces = MediaType.APPLICATION_JSON, headers = "Authorization") | ||
| public String resendConsent(@Param(value = "{\"mobNo\":\"String\"}") @RequestBody String requestOBJ) { | ||
| logger.info(requestOBJ.toString()); |
Check notice
Code scanning / SonarCloud
Logging should not be vulnerable to injection attacks Low
| @Operation(summary = "Send OTP") | ||
| @RequestMapping(value = "/sendOTP", method = RequestMethod.POST, consumes = MediaType.APPLICATION_JSON, produces = MediaType.APPLICATION_JSON, headers = "Authorization") | ||
| public String sendOTP(@Param(value = "{\"mobNo\":\"String\"}") @RequestBody String requestOBJ) { | ||
| logger.info(requestOBJ.toString()); |
Check notice
Code scanning / SonarCloud
Logging should not be vulnerable to injection attacks Low
| @Operation(summary = "Resend OTP") | ||
| @RequestMapping(value = "/resendOTP", method = RequestMethod.POST, consumes = MediaType.APPLICATION_JSON, produces = MediaType.APPLICATION_JSON, headers = "Authorization") | ||
| public String resendOTP(@Param(value = "{\"mobNo\":\"String\"}") @RequestBody String requestOBJ) { | ||
| logger.info(requestOBJ.toString()); |
Check notice
Code scanning / SonarCloud
Logging should not be vulnerable to injection attacks Low
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (11)
src/main/environment/common_ci.properties (1)
43-49: Align environment variable namingThe property
firebase.enabledis populated from@env.FIREBASE_ENABLE@. For consistency and clarity, consider renaming the environment variable toFIREBASE_ENABLEDto match the property key format.src/main/java/com/iemr/common/CommonApplication.java (1)
24-27: Remove unused imports.The Firebase-related imports, ClassPathResource, and IOException are not used in this class. Consider removing them to keep the code clean.
-import com.google.auth.oauth2.GoogleCredentials; -import com.google.firebase.FirebaseApp; -import com.google.firebase.FirebaseOptions; -import com.google.firebase.messaging.FirebaseMessaging; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.web.servlet.support.SpringBootServletInitializer; import org.springframework.context.annotation.Bean; -import org.springframework.core.io.ClassPathResource; import org.springframework.data.redis.connection.RedisConnectionFactory;import com.iemr.common.data.users.User; import com.iemr.common.utils.IEMRApplBeans; -import java.io.IOException;Also applies to: 33-33, 44-44
src/main/java/com/iemr/common/data/beneficiaryConsent/BeneficiaryConsentRequest.java (1)
7-10: Consider adding validation annotations for data integrity.The request fields lack validation constraints which could lead to invalid data being processed. Consider adding appropriate validation annotations.
+import javax.validation.constraints.NotBlank; +import javax.validation.constraints.Pattern; + @Data public class BeneficiaryConsentRequest { + @NotBlank(message = "Mobile number is required") + @Pattern(regexp = "^[0-9]{10}$", message = "Mobile number must be 10 digits") private String mobNo; + @NotBlank(message = "OTP is required") private String otp; + @NotBlank(message = "Username is required") private String userName; + @NotBlank(message = "Designation is required") private String designation;src/main/java/com/iemr/common/service/beneficiaryOTPHandler/BeneficiaryOTPHandler.java (2)
4-4: Remove unused import.The
OTPRequestParsorimport is not used in this interface and should be removed.-import com.iemr.common.data.otp.OTPRequestParsor;
8-12: Consider standardizing return types for consistency.The interface has inconsistent return types (
StringvsJSONObject). Consider using a consistent response wrapper or standardizing on a single return type for better API consistency.src/main/java/com/iemr/common/controller/firebaseNotification/FirebaseNotificationController.java (1)
13-13: Remove unused logger field.The logger field is declared but never used in the controller. Either use it for logging or remove it to reduce clutter.
- final Logger logger = LoggerFactory.getLogger(this.getClass().getName());src/main/java/com/iemr/common/config/firebase/FirebaseMessagingConfig.java (3)
30-32: Consider logging configuration status instead of throwing exception.Throwing an exception when Firebase is disabled might not be the best approach if the application should continue to function without Firebase. Consider logging the status and returning a no-op implementation instead.
if (!firebaseEnabled) { + logger.info("Firebase is disabled, skipping Firebase messaging configuration"); - throw new IllegalStateException("Firebase is disabled"); + return null; // or return a no-op implementation }
43-45: Improve error message specificity.The error message could be more helpful by indicating what configuration is expected.
-throw new IllegalStateException("No Firebase credentials provided"); +throw new IllegalStateException("No Firebase credentials provided. Please configure either 'firebase.credential-file' or 'firebase.credential-base64' property");
51-53: Add logging for Firebase app initialization.Consider adding logging to track Firebase app initialization status for better debugging and monitoring.
+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + @Configuration public class FirebaseMessagingConfig { + private static final Logger logger = LoggerFactory.getLogger(FirebaseMessagingConfig.class); + // ... existing code ... FirebaseApp firebaseApp = FirebaseApp.getApps().isEmpty() - ? FirebaseApp.initializeApp(options) + ? FirebaseApp.initializeApp(options, "common-api") : FirebaseApp.getInstance(); + + logger.info("Firebase app initialized: {}", firebaseApp.getName());src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java (1)
15-15: Remove unused logger or add proper logging.The logger is declared but never used. Either utilize it for error logging or remove it.
- final Logger logger = LoggerFactory.getLogger(this.getClass().getName());src/main/java/com/iemr/common/controller/beneficiaryConsent/BeneficiaryConsentController.java (1)
6-6: Remove unused import.The
OTPHandlerimport is not used in this class and should be removed.-import com.iemr.common.service.otp.OTPHandler;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
pom.xml(1 hunks)src/main/environment/common_ci.properties(1 hunks)src/main/java/com/iemr/common/CommonApplication.java(3 hunks)src/main/java/com/iemr/common/config/firebase/FirebaseMessagingConfig.java(1 hunks)src/main/java/com/iemr/common/controller/beneficiaryConsent/BeneficiaryConsentController.java(1 hunks)src/main/java/com/iemr/common/controller/firebaseNotification/FirebaseNotificationController.java(1 hunks)src/main/java/com/iemr/common/controller/otp/OTPGateway.java(2 hunks)src/main/java/com/iemr/common/data/beneficiaryConsent/BeneficiaryConsentRequest.java(1 hunks)src/main/java/com/iemr/common/dto/identity/CommonIdentityDTO.java(1 hunks)src/main/java/com/iemr/common/model/beneficiary/BeneficiaryModel.java(2 hunks)src/main/java/com/iemr/common/model/notification/NotificationMessage.java(1 hunks)src/main/java/com/iemr/common/service/beneficiary/RegisterBenificiaryServiceImpl.java(2 hunks)src/main/java/com/iemr/common/service/beneficiaryOTPHandler/BeneficiaryOTPHandler.java(1 hunks)src/main/java/com/iemr/common/service/beneficiaryOTPHandler/BeneficiaryOTPHandlerImpl.java(1 hunks)src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java(1 hunks)src/main/java/com/iemr/common/service/notification/NotificationService.java(1 hunks)src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
- GitHub Check: Build
🔇 Additional comments (8)
src/main/java/com/iemr/common/service/notification/NotificationService.java (1)
58-58: No-op change, blank line addedThe added blank line at the end of the interface declaration has no functional impact.
src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java (1)
62-62: No-op change, blank line addedA blank line was inserted after the logging statement. This is purely cosmetic and does not alter any logic.
pom.xml (1)
114-119: Add Firebase Admin SDK dependencyThe Firebase Admin SDK (
firebase-admin:9.4.3) is correctly added to support FCM integration. Ensure this version is compatible with Spring Boot 3.2.2 and monitor for newer stable releases to keep dependencies up-to-date.src/main/java/com/iemr/common/model/notification/NotificationMessage.java (1)
1-15: New DTO for Firebase notificationsThe
NotificationMessageclass cleanly encapsulates the notification payload using Lombok’s@Data. All required fields are present and appropriately typed.src/main/java/com/iemr/common/controller/otp/OTPGateway.java (2)
56-56: Good logging additions for debugging OTP flows.The request logging will help trace OTP operations during troubleshooting.
Also applies to: 103-103
64-65: ```shell
#!/bin/bashLocate OTPGateway methods and inspect return values and conditions
1. Find all occurrences of sendOTP and show context
echo "=== sendOTP references ==="
rg -n 'sendOTP' src
for file in $(rg -l 'sendOTP' src); do
echo -e "\n--- Context in $file ---"
rg -n -C5 'sendOTP' "$file"
done2. Find all occurrences of resendOTP and show context
echo -e "\n=== resendOTP references ==="
rg -n 'resendOTP' src
for file in $(rg -l 'resendOTP' src); do
echo -e "\n--- Context in $file ---"
rg -n -C5 'resendOTP' "$file"
done3. Show previous exact match check
echo -e "\n=== Previous equalsIgnoreCase usage in OTPGateway.java ==="
rg -n -C3 'equalsIgnoreCase' src/main/java/com/iemr/common/controller/otp/OTPGateway.java4. Show new contains("otp") check
echo -e "\n=== New contains("otp") usage in OTPGateway.java ==="
rg -n -C3 'contains("otp")' src/main/java/com/iemr/common/controller/otp/OTPGateway.java</details> <details> <summary>src/main/java/com/iemr/common/service/beneficiary/RegisterBenificiaryServiceImpl.java (1)</summary> `167-167`: **Good addition of debug logging.** The logging will help trace beneficiary registration operations. </details> <details> <summary>src/main/java/com/iemr/common/service/beneficiaryOTPHandler/BeneficiaryOTPHandlerImpl.java (1)</summary> `132-133`: **Approve secure OTP generation implementation.** The use of `SecureRandom.getInstanceStrong()` and 6-digit OTP generation provides good security practices for OTP generation. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| identityDTO.setIsConsent(beneficiaryModel.getIsConsent()); | ||
| identityDTO.setFaceEmbedding(beneficiaryModel.getFaceEmbedding()); | ||
| identityDTO.setEmergencyRegistration(beneficiaryModel.isEmergencyRegistration()); | ||
| identityDTO.setIsConsent(beneficiaryModel.getIsConsent()); |
There was a problem hiding this comment.
Remove duplicate assignment.
The isConsent field is being set twice consecutively from the same source, which is redundant.
identityDTO.setIsConsent(beneficiaryModel.getIsConsent());
identityDTO.setFaceEmbedding(beneficiaryModel.getFaceEmbedding());
identityDTO.setEmergencyRegistration(beneficiaryModel.isEmergencyRegistration());
-identityDTO.setIsConsent(beneficiaryModel.getIsConsent());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| identityDTO.setIsConsent(beneficiaryModel.getIsConsent()); | |
| identityDTO.setFaceEmbedding(beneficiaryModel.getFaceEmbedding()); | |
| identityDTO.setEmergencyRegistration(beneficiaryModel.isEmergencyRegistration()); | |
| identityDTO.setIsConsent(beneficiaryModel.getIsConsent()); | |
| identityDTO.setIsConsent(beneficiaryModel.getIsConsent()); | |
| identityDTO.setFaceEmbedding(beneficiaryModel.getFaceEmbedding()); | |
| identityDTO.setEmergencyRegistration(beneficiaryModel.isEmergencyRegistration()); |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/service/beneficiary/RegisterBenificiaryServiceImpl.java
around lines 172 to 175, there is a duplicate assignment of the isConsent field
on identityDTO from beneficiaryModel. Remove the redundant second call to
identityDTO.setIsConsent to avoid unnecessary repetition.
| @Expose | ||
| private Boolean isConsent=false; |
There was a problem hiding this comment.
Fix duplicate field declaration.
Two isConsent fields are declared with identical names, types, and initialization values. This will cause compilation issues or unexpected behavior.
@Expose
private List<BenPhoneMapModel> parentBenPhoneMaps;
-@Expose
-private Boolean isConsent=false;
@Expose
private String beneficiaryID;
@Expose
private Short titleId;
@Expose
private String titleName;
@Expose
private TitleModel m_title;
@Expose
private String firstName;
@Expose
private Boolean isConsent=false;Also applies to: 94-95
🤖 Prompt for AI Agents
In src/main/java/com/iemr/common/model/beneficiary/BeneficiaryModel.java at
lines 80-81 and 94-95, there are duplicate declarations of the field isConsent
with the same name, type, and initialization. Remove one of the duplicate
declarations in each case to ensure only a single isConsent field exists,
preventing compilation errors and unexpected behavior.
| private Integer beneficiaryRegId; | ||
| private Integer communityId; | ||
| private String community; | ||
| private Boolean isConsent=false; |
There was a problem hiding this comment.
Fix duplicate field declaration.
Two isConsent fields are declared - one initialized to false and one uninitialized. This will cause compilation issues or unexpected behavior.
private String community;
-private Boolean isConsent=false;
private Timestamp dob;
private Integer ageAtMarriage;
private Integer educationId;
private String education;
private Boolean emergencyRegistration = false;
-private Boolean isConsent;
+private Boolean isConsent = false;Also applies to: 64-64
🤖 Prompt for AI Agents
In src/main/java/com/iemr/common/dto/identity/CommonIdentityDTO.java at lines 57
and 64, there are duplicate declarations of the field isConsent, one initialized
to false and one uninitialized. Remove one of these declarations to eliminate
the duplicate field error and ensure only a single isConsent field exists in the
class.
| @Data | ||
| public class BeneficiaryConsentRequest { | ||
| private String mobNo; | ||
| private int otp; |
There was a problem hiding this comment.
Change OTP field type to String to preserve leading zeros.
Using int for OTP can cause issues if the OTP starts with zero, as leading zeros will be lost. OTPs should be treated as strings to preserve their exact format.
- private int otp;
+ private String otp;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private int otp; | |
| - private int otp; | |
| + private String otp; |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/data/beneficiaryConsent/BeneficiaryConsentRequest.java
at line 8, change the type of the otp field from int to String to ensure leading
zeros are preserved and the OTP format remains accurate.
| public String sendOTP(BeneficiaryConsentRequest obj) throws Exception; | ||
|
|
||
| public JSONObject validateOTP(BeneficiaryConsentRequest obj) throws Exception; | ||
|
|
||
| public String resendOTP(BeneficiaryConsentRequest obj) throws Exception; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace generic Exception with specific exception types.
Using generic Exception makes it difficult for clients to handle specific error scenarios. Consider using specific exception types or creating custom exceptions for different failure modes.
- public String sendOTP(BeneficiaryConsentRequest obj) throws Exception;
+ public String sendOTP(BeneficiaryConsentRequest obj) throws OTPGenerationException, SMSDeliveryException;
- public JSONObject validateOTP(BeneficiaryConsentRequest obj) throws Exception;
+ public JSONObject validateOTP(BeneficiaryConsentRequest obj) throws OTPValidationException;
- public String resendOTP(BeneficiaryConsentRequest obj) throws Exception;
+ public String resendOTP(BeneficiaryConsentRequest obj) throws OTPGenerationException, SMSDeliveryException;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String sendOTP(BeneficiaryConsentRequest obj) throws Exception; | |
| public JSONObject validateOTP(BeneficiaryConsentRequest obj) throws Exception; | |
| public String resendOTP(BeneficiaryConsentRequest obj) throws Exception; | |
| public String sendOTP(BeneficiaryConsentRequest obj) throws OTPGenerationException, SMSDeliveryException; | |
| public JSONObject validateOTP(BeneficiaryConsentRequest obj) throws OTPValidationException; | |
| public String resendOTP(BeneficiaryConsentRequest obj) throws OTPGenerationException, SMSDeliveryException; |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/service/beneficiaryOTPHandler/BeneficiaryOTPHandler.java
around lines 8 to 12, replace the generic Exception in the method signatures of
sendOTP, validateOTP, and resendOTP with more specific exception types relevant
to the possible failure scenarios. Identify the specific exceptions these
methods can throw or define custom exceptions to clearly communicate error
conditions, and update the method signatures accordingly to improve error
handling clarity.
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RequestMethod; | ||
|
|
||
| public class BeneficiaryConsentController { |
There was a problem hiding this comment.
Add missing @RestController annotation.
The class is missing the @RestController annotation, which is required for Spring to recognize it as a REST controller and handle HTTP requests properly.
+@RestController
public class BeneficiaryConsentController {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public class BeneficiaryConsentController { | |
| @RestController | |
| public class BeneficiaryConsentController { |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/controller/beneficiaryConsent/BeneficiaryConsentController.java
at line 20, the class lacks the @RestController annotation. Add the
@RestController annotation above the class declaration to ensure Spring
recognizes this class as a REST controller and properly handles HTTP requests.
| if (success.contains("otp")) | ||
| response.setResponse(success); | ||
| else | ||
| response.setError(500, "failure"); |
There was a problem hiding this comment.
Fix success condition logic mismatch.
The success condition checks for "otp" in the response, but the service method returns "success". This will cause all successful OTP sends to be treated as failures.
- if (success.contains("otp"))
+ if (success.equals("success"))
response.setResponse(success);
else
response.setError(500, "failure");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (success.contains("otp")) | |
| response.setResponse(success); | |
| else | |
| response.setError(500, "failure"); | |
| if (success.equals("success")) | |
| response.setResponse(success); | |
| else | |
| response.setError(500, "failure"); |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/controller/beneficiaryConsent/BeneficiaryConsentController.java
around lines 38 to 41, the condition incorrectly checks if the success string
contains "otp" instead of "success". Update the condition to check if the
success string contains "success" to correctly identify successful OTP sends and
avoid treating them as failures.
| OTPRequestParsor obj = InputMapper.gson().fromJson(requestOBJ, OTPRequestParsor.class); | ||
|
|
||
| String success = beneficiaryOTPHandler.resendOTP(obj); |
There was a problem hiding this comment.
Fix parameter type inconsistency in resendConsent method.
Similar to validateConsent, this method uses OTPRequestParsor but the service method resendOTP expects BeneficiaryConsentRequest.
- OTPRequestParsor obj = InputMapper.gson().fromJson(requestOBJ, OTPRequestParsor.class);
+ BeneficiaryConsentRequest obj = InputMapper.gson().fromJson(requestOBJ, BeneficiaryConsentRequest.class);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| OTPRequestParsor obj = InputMapper.gson().fromJson(requestOBJ, OTPRequestParsor.class); | |
| String success = beneficiaryOTPHandler.resendOTP(obj); | |
| BeneficiaryConsentRequest obj = InputMapper.gson().fromJson(requestOBJ, BeneficiaryConsentRequest.class); | |
| String success = beneficiaryOTPHandler.resendOTP(obj); |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/controller/beneficiaryConsent/BeneficiaryConsentController.java
around lines 80 to 82, the resendConsent method incorrectly parses the input
JSON into an OTPRequestParsor object, but the resendOTP service method expects a
BeneficiaryConsentRequest object. To fix this, change the parsing to convert the
requestOBJ JSON into a BeneficiaryConsentRequest instance before passing it to
resendOTP, ensuring the parameter types are consistent.
| if (success.contains("otp")) | ||
| response.setResponse(success); | ||
| else | ||
| response.setError(500, "failure"); |
There was a problem hiding this comment.
Fix success condition logic in resendConsent method.
Same issue as in sendConsent - the success condition logic is incorrect.
- if (success.contains("otp"))
+ if (success.equals("success"))
response.setResponse(success);
else
response.setError(500, "failure");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (success.contains("otp")) | |
| response.setResponse(success); | |
| else | |
| response.setError(500, "failure"); | |
| if (success.equals("success")) | |
| response.setResponse(success); | |
| else | |
| response.setError(500, "failure"); |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/controller/beneficiaryConsent/BeneficiaryConsentController.java
around lines 85 to 88, the success condition logic in the resendConsent method
is incorrect as it only checks if the success string contains "otp". Update the
condition to correctly reflect the success criteria used in the sendConsent
method, ensuring that the response is set to success only when the operation
truly succeeds; otherwise, set the error response appropriately.
| OTPRequestParsor obj = InputMapper.gson().fromJson(requestOBJ, OTPRequestParsor.class); | ||
|
|
||
| JSONObject responseOBJ = beneficiaryOTPHandler.validateOTP(obj); |
There was a problem hiding this comment.
Fix parameter type inconsistency causing method signature mismatch.
The validateConsent method uses OTPRequestParsor but the service method validateOTP expects BeneficiaryConsentRequest. This will cause a compilation error or runtime issues.
- OTPRequestParsor obj = InputMapper.gson().fromJson(requestOBJ, OTPRequestParsor.class);
+ BeneficiaryConsentRequest obj = InputMapper.gson().fromJson(requestOBJ, BeneficiaryConsentRequest.class);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| OTPRequestParsor obj = InputMapper.gson().fromJson(requestOBJ, OTPRequestParsor.class); | |
| JSONObject responseOBJ = beneficiaryOTPHandler.validateOTP(obj); | |
| BeneficiaryConsentRequest obj = InputMapper.gson().fromJson(requestOBJ, BeneficiaryConsentRequest.class); | |
| JSONObject responseOBJ = beneficiaryOTPHandler.validateOTP(obj); |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/controller/beneficiaryConsent/BeneficiaryConsentController.java
around lines 57 to 59, the parameter type passed to the validateOTP method is
OTPRequestParsor, but validateOTP expects a BeneficiaryConsentRequest object.
Change the parameter type to BeneficiaryConsentRequest by parsing the requestOBJ
into BeneficiaryConsentRequest instead of OTPRequestParsor before calling
validateOTP to fix the method signature mismatch.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
src/main/java/com/iemr/common/controller/firebaseNotification/FirebaseNotificationController.java (2)
44-47: Add error handling and input validation.The sendNotification endpoint still lacks proper error handling and input validation as previously identified. This could lead to runtime exceptions and poor user experience.
44-47: Add input validation and proper error handling.The endpoint lacks essential input validation and error handling, which could lead to runtime exceptions and poor user experience.
src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java (4)
74-74: Use the autowired FirebaseMessaging instance instead of getInstance().The code still uses
FirebaseMessaging.getInstance()instead of the autowiredfirebaseMessagingfield, which bypasses dependency injection.-String response = FirebaseMessaging.getInstance().send(message); +String response = firebaseMessaging.send(message);
66-81: Improve error handling and add input validation.The sendNotification method still lacks proper input validation and error logging as previously identified. The generic error message doesn't help with debugging.
74-74: Use the autowired FirebaseMessaging instance instead of getInstance().The code bypasses dependency injection by using
FirebaseMessaging.getInstance()instead of the autowiredfirebaseMessagingfield.
66-81: Add comprehensive input validation and improve error handling.The method lacks essential input validation and provides insufficient error details for debugging.
🧹 Nitpick comments (3)
src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java (2)
89-89: Replace mixed language comments with English.Comments contain Hindi text mixed with English, which affects code readability and maintainability in an international development environment.
- // User token exist karta hai => update karna hai + // User token exists => update it- // User token nahi mila => naya insert karna hai + // User token not found => insert new one- .map(UserTokenData::getToken) // mil gaya to token nikalo - .orElse(null); // nah + .map(UserTokenData::getToken) // if found, extract token + .orElse(null); // if not found, return nullAlso applies to: 94-94, 109-111
89-89: Use consistent language in code comments.The Hindi comments reduce code readability for international development teams. Consider using English consistently throughout the codebase.
- // User token exist karta hai => update karna hai + // User token exists => update it- // User token nahi mila => naya insert karna hai + // User token not found => insert new recordAlso applies to: 94-94
src/main/java/com/iemr/common/controller/firebaseNotification/FirebaseNotificationController.java (1)
54-58: Improve response consistency and add logging.While this endpoint properly handles exceptions with
IEMRException, consider returningResponseEntity<String>for consistency with HTTP status codes and add logging for better observability.@RequestMapping(value = "getToken",method = RequestMethod.GET,headers = "Authorization") -public String getUserToken() throws IEMRException { +public ResponseEntity<String> getUserToken() throws IEMRException { + try { + String token = firebaseNotificationService.getUserToken(); + if (token != null) { + logger.info("Token retrieved successfully"); + return ResponseEntity.ok(token); + } else { + logger.warn("No token found for user"); + return ResponseEntity.notFound().build(); + } + } catch (IEMRException e) { + logger.error("Error retrieving user token: ", e); + throw e; // Re-throw to maintain existing behavior + } - return firebaseNotificationService.getUserToken(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/controller/firebaseNotification/FirebaseNotificationController.java(1 hunks)src/main/java/com/iemr/common/data/userToken/UserTokenData.java(1 hunks)src/main/java/com/iemr/common/model/notification/NotificationMessage.java(1 hunks)src/main/java/com/iemr/common/model/notification/UserToken.java(1 hunks)src/main/java/com/iemr/common/repo/userToken/UserTokenRepo.java(1 hunks)src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/main/java/com/iemr/common/model/notification/UserToken.java
- src/main/java/com/iemr/common/repo/userToken/UserTokenRepo.java
- src/main/java/com/iemr/common/data/userToken/UserTokenData.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/iemr/common/model/notification/NotificationMessage.java
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Analyze (java)
- GitHub Check: Package-test
- GitHub Check: Build
🔇 Additional comments (1)
src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java (1)
105-112: ```shell
#!/bin/bashLocate and inspect the UserTokenData entity to verify the type of its ID field
1. Find the file
file=$(find . -type f -name "UserTokenData.java" | head -n1)
echo "Found file: $file"2. If found, print the class declaration and its field definitions (first 200 lines)
if [[ -n "$file" ]]; then
echo "----- Begin UserTokenData.java -----"
sed -n '1,200p' "$file"
echo "----- End UserTokenData.java -----"
else
echo "UserTokenData.java not found in repository."
fi</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| @RequestMapping(value = "updateToken",method = RequestMethod.POST) | ||
| public String updateToken(@RequestBody UserToken userToken){ | ||
| return firebaseNotificationService.updateToken(userToken); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and input validation for updateToken endpoint.
Similar to the sendNotification endpoint, this method lacks error handling and input validation. Consider returning proper HTTP status codes and handling potential exceptions.
@RequestMapping(value = "updateToken",method = RequestMethod.POST)
-public String updateToken(@RequestBody UserToken userToken){
+public ResponseEntity<String> updateToken(@Valid @RequestBody UserToken userToken){
+ try {
+ if (userToken == null || userToken.getUserId() == null || userToken.getToken() == null) {
+ return ResponseEntity.badRequest().body("User ID and token are required");
+ }
+ String result = firebaseNotificationService.updateToken(userToken);
+ return ResponseEntity.ok(result);
+ } catch (Exception e) {
+ logger.error("Error updating token: ", e);
+ return ResponseEntity.internalServerError().body("Failed to update token");
+ }
- return firebaseNotificationService.updateToken(userToken);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @RequestMapping(value = "updateToken",method = RequestMethod.POST) | |
| public String updateToken(@RequestBody UserToken userToken){ | |
| return firebaseNotificationService.updateToken(userToken); | |
| } | |
| @RequestMapping(value = "updateToken", method = RequestMethod.POST) | |
| public ResponseEntity<String> updateToken(@Valid @RequestBody UserToken userToken) { | |
| try { | |
| if (userToken == null || userToken.getUserId() == null || userToken.getToken() == null) { | |
| return ResponseEntity.badRequest().body("User ID and token are required"); | |
| } | |
| String result = firebaseNotificationService.updateToken(userToken); | |
| return ResponseEntity.ok(result); | |
| } catch (Exception e) { | |
| logger.error("Error updating token: ", e); | |
| return ResponseEntity.internalServerError().body("Failed to update token"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/controller/firebaseNotification/FirebaseNotificationController.java
around lines 49 to 52, the updateToken method lacks error handling and input
validation. Add validation to check the userToken input for null or invalid
fields before processing. Wrap the service call in a try-catch block to handle
exceptions and return appropriate HTTP status codes, such as 400 for bad input
and 500 for server errors, using ResponseEntity to provide meaningful responses.
🛠️ Refactor suggestion
Add input validation and error handling for consistency.
Similar to the sendNotificationByToken method, this endpoint lacks input validation and error handling. Consider applying the same improvements for consistency.
@RequestMapping(value = "updateToken",method = RequestMethod.POST)
-public String updateToken(@RequestBody UserToken userToken){
+public ResponseEntity<String> updateToken(@Valid @RequestBody UserToken userToken){
+ try {
+ if (userToken == null) {
+ return ResponseEntity.badRequest().body("User token is required");
+ }
String result = firebaseNotificationService.updateToken(userToken);
- return firebaseNotificationService.updateToken(userToken);
+ return ResponseEntity.ok(result);
+ } catch (Exception e) {
+ logger.error("Error updating token: ", e);
+ return ResponseEntity.internalServerError().body("Failed to update token");
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/controller/firebaseNotification/FirebaseNotificationController.java
around lines 49 to 52, the updateToken method lacks input validation and error
handling. Add validation to check if the userToken object and its required
fields are not null or empty before proceeding. Wrap the service call in a
try-catch block to handle potential exceptions and return appropriate error
responses or messages for consistency with the sendNotificationByToken method.
| @RequestMapping(value = "getToken",method = RequestMethod.GET,headers = "Authorization") | ||
| public String getUserToken() throws IEMRException { | ||
|
|
||
| return firebaseNotificationService.getUserToken(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for getUserToken endpoint.
The getUserToken method throws IEMRException but doesn't handle it properly at the controller level. Consider wrapping in try-catch and returning appropriate HTTP responses.
@RequestMapping(value = "getToken",method = RequestMethod.GET,headers = "Authorization")
-public String getUserToken() throws IEMRException {
-
- return firebaseNotificationService.getUserToken();
+public ResponseEntity<String> getUserToken() {
+ try {
+ String token = firebaseNotificationService.getUserToken();
+ if (token == null) {
+ return ResponseEntity.notFound().build();
+ }
+ return ResponseEntity.ok(token);
+ } catch (IEMRException e) {
+ logger.error("Error retrieving user token: ", e);
+ return ResponseEntity.internalServerError().body("Failed to retrieve token");
+ }
}🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/controller/firebaseNotification/FirebaseNotificationController.java
around lines 54 to 58, the getUserToken method declares throwing IEMRException
but does not handle it within the controller. Modify the method to wrap the call
to firebaseNotificationService.getUserToken() in a try-catch block, catch
IEMRException, and return an appropriate HTTP response (e.g., ResponseEntity
with error status and message) instead of letting the exception propagate.
| public String getUserToken() throws IEMRException { | ||
| HttpServletRequest requestHeader = ((ServletRequestAttributes) RequestContextHolder.getRequestAttributes()) | ||
| .getRequest(); | ||
| String jwtTokenFromCookie = cookieUtil.getJwtTokenFromCookie(requestHeader); | ||
| return userTokenRepo.findById(Integer.parseInt(jwtUtil.extractUserId(jwtTokenFromCookie))) // because your userId is Long in DB | ||
| .map(UserTokenData::getToken) // mil gaya to token nikalo | ||
| .orElse(null); // nah | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and add null checks in getUserToken method.
The method lacks proper error handling for potential JWT parsing failures and database access issues.
public String getUserToken() throws IEMRException {
- HttpServletRequest requestHeader = ((ServletRequestAttributes) RequestContextHolder.getRequestAttributes())
- .getRequest();
- String jwtTokenFromCookie = cookieUtil.getJwtTokenFromCookie(requestHeader);
- return userTokenRepo.findById(Integer.parseInt(jwtUtil.extractUserId(jwtTokenFromCookie)))
- .map(UserTokenData::getToken)
- .orElse(null);
+ try {
+ ServletRequestAttributes attributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
+ if (attributes == null) {
+ throw new IEMRException("No request context available");
+ }
+
+ HttpServletRequest requestHeader = attributes.getRequest();
+ String jwtTokenFromCookie = cookieUtil.getJwtTokenFromCookie(requestHeader);
+
+ if (jwtTokenFromCookie == null) {
+ throw new IEMRException("JWT token not found in cookie");
+ }
+
+ String userIdStr = jwtUtil.extractUserId(jwtTokenFromCookie);
+ if (userIdStr == null) {
+ throw new IEMRException("User ID not found in JWT token");
+ }
+
+ Integer userId = Integer.parseInt(userIdStr);
+ return userTokenRepo.findById(userId)
+ .map(UserTokenData::getToken)
+ .orElse(null);
+ } catch (NumberFormatException e) {
+ logger.error("Invalid user ID format in JWT token", e);
+ throw new IEMRException("Invalid user ID format");
+ } catch (Exception e) {
+ logger.error("Error retrieving user token", e);
+ throw new IEMRException("Failed to retrieve user token");
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String getUserToken() throws IEMRException { | |
| HttpServletRequest requestHeader = ((ServletRequestAttributes) RequestContextHolder.getRequestAttributes()) | |
| .getRequest(); | |
| String jwtTokenFromCookie = cookieUtil.getJwtTokenFromCookie(requestHeader); | |
| return userTokenRepo.findById(Integer.parseInt(jwtUtil.extractUserId(jwtTokenFromCookie))) // because your userId is Long in DB | |
| .map(UserTokenData::getToken) // mil gaya to token nikalo | |
| .orElse(null); // nah | |
| } | |
| public String getUserToken() throws IEMRException { | |
| try { | |
| ServletRequestAttributes attributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes(); | |
| if (attributes == null) { | |
| throw new IEMRException("No request context available"); | |
| } | |
| HttpServletRequest requestHeader = attributes.getRequest(); | |
| String jwtTokenFromCookie = cookieUtil.getJwtTokenFromCookie(requestHeader); | |
| if (jwtTokenFromCookie == null) { | |
| throw new IEMRException("JWT token not found in cookie"); | |
| } | |
| String userIdStr = jwtUtil.extractUserId(jwtTokenFromCookie); | |
| if (userIdStr == null) { | |
| throw new IEMRException("User ID not found in JWT token"); | |
| } | |
| Integer userId = Integer.parseInt(userIdStr); | |
| return userTokenRepo.findById(userId) | |
| .map(UserTokenData::getToken) | |
| .orElse(null); | |
| } catch (NumberFormatException e) { | |
| logger.error("Invalid user ID format in JWT token", e); | |
| throw new IEMRException("Invalid user ID format"); | |
| } catch (Exception e) { | |
| logger.error("Error retrieving user token", e); | |
| throw new IEMRException("Failed to retrieve user token"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java
around lines 105 to 112, the getUserToken method lacks error handling for JWT
parsing and database access. Add try-catch blocks to handle exceptions from
jwtUtil.extractUserId and userTokenRepo.findById calls. Also, include null
checks for the JWT token extracted from the cookie before parsing and handle
cases where the user ID or token is not found, returning null or throwing a
custom exception as appropriate.
| public String updateToken(UserToken userToken) { | ||
| Optional<UserTokenData> existingTokenData = userTokenRepo.findById(userToken.getUserId()); | ||
|
|
||
| UserTokenData userTokenData; | ||
|
|
||
| if (existingTokenData.isPresent()) { | ||
| // User token exist karta hai => update karna hai | ||
| userTokenData = existingTokenData.get(); | ||
| userTokenData.setToken(userToken.getToken()); | ||
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | ||
| } else { | ||
| // User token nahi mila => naya insert karna hai | ||
| userTokenData = new UserTokenData(); | ||
| userTokenData.setUserId(userToken.getUserId()); | ||
| userTokenData.setToken(userToken.getToken()); | ||
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | ||
| } | ||
|
|
||
| userTokenRepo.save(userTokenData); | ||
| return "Save Successfully"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and improve error handling in updateToken method.
The updateToken method lacks input validation and proper error handling, which could lead to runtime exceptions.
public String updateToken(UserToken userToken) {
+ if (userToken == null) {
+ logger.error("UserToken is null");
+ return "Error: User token data is required";
+ }
+
+ if (userToken.getUserId() == null || userToken.getToken() == null || userToken.getToken().trim().isEmpty()) {
+ logger.error("Invalid user token data - userId or token is missing");
+ return "Error: User ID and token are required";
+ }
+
+ try {
Optional<UserTokenData> existingTokenData = userTokenRepo.findById(userToken.getUserId());
// ... rest of the method
userTokenRepo.save(userTokenData);
+ logger.info("Token updated successfully for user: {}", userToken.getUserId());
return "Save Successfully";
+ } catch (Exception e) {
+ logger.error("Error updating token for user: {}", userToken.getUserId(), e);
+ return "Error updating token: " + e.getMessage();
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String updateToken(UserToken userToken) { | |
| Optional<UserTokenData> existingTokenData = userTokenRepo.findById(userToken.getUserId()); | |
| UserTokenData userTokenData; | |
| if (existingTokenData.isPresent()) { | |
| // User token exist karta hai => update karna hai | |
| userTokenData = existingTokenData.get(); | |
| userTokenData.setToken(userToken.getToken()); | |
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | |
| } else { | |
| // User token nahi mila => naya insert karna hai | |
| userTokenData = new UserTokenData(); | |
| userTokenData.setUserId(userToken.getUserId()); | |
| userTokenData.setToken(userToken.getToken()); | |
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | |
| } | |
| userTokenRepo.save(userTokenData); | |
| return "Save Successfully"; | |
| } | |
| public String updateToken(UserToken userToken) { | |
| if (userToken == null) { | |
| logger.error("UserToken is null"); | |
| return "Error: User token data is required"; | |
| } | |
| if (userToken.getUserId() == null | |
| || userToken.getToken() == null | |
| || userToken.getToken().trim().isEmpty()) { | |
| logger.error("Invalid user token data - userId or token is missing"); | |
| return "Error: User ID and token are required"; | |
| } | |
| try { | |
| Optional<UserTokenData> existingTokenData = userTokenRepo.findById(userToken.getUserId()); | |
| UserTokenData userTokenData; | |
| if (existingTokenData.isPresent()) { | |
| // User token exist karta hai => update karna hai | |
| userTokenData = existingTokenData.get(); | |
| userTokenData.setToken(userToken.getToken()); | |
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | |
| } else { | |
| // User token nahi mila => naya insert karna hai | |
| userTokenData = new UserTokenData(); | |
| userTokenData.setUserId(userToken.getUserId()); | |
| userTokenData.setToken(userToken.getToken()); | |
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | |
| } | |
| userTokenRepo.save(userTokenData); | |
| logger.info("Token updated successfully for user: {}", userToken.getUserId()); | |
| return "Save Successfully"; | |
| } catch (Exception e) { | |
| logger.error("Error updating token for user: {}", userToken.getUserId(), e); | |
| return "Error updating token: " + e.getMessage(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java
between lines 83 and 103, the updateToken method lacks input validation and
error handling. Add checks to validate that the userToken and its userId and
token fields are not null or empty before proceeding. Wrap the database
operations in a try-catch block to handle potential exceptions, and return
meaningful error messages or throw custom exceptions as appropriate to improve
robustness.
🛠️ Refactor suggestion
Add input validation and error handling for database operations.
The method lacks input validation and proper error handling for database operations, which could lead to runtime exceptions.
public String updateToken(UserToken userToken) {
+ if (userToken == null || userToken.getUserId() == null || userToken.getToken() == null) {
+ logger.error("Invalid user token data provided");
+ return "Error: Invalid user token data";
+ }
+
+ try {
Optional<UserTokenData> existingTokenData = userTokenRepo.findById(userToken.getUserId());
// ... rest of the logic
userTokenRepo.save(userTokenData);
- return "Save Successfully";
+ logger.info("Token updated successfully for user: {}", userToken.getUserId());
+ return "Token saved successfully";
+ } catch (Exception e) {
+ logger.error("Error saving user token for user: {}", userToken.getUserId(), e);
+ return "Error saving token";
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String updateToken(UserToken userToken) { | |
| Optional<UserTokenData> existingTokenData = userTokenRepo.findById(userToken.getUserId()); | |
| UserTokenData userTokenData; | |
| if (existingTokenData.isPresent()) { | |
| // User token exist karta hai => update karna hai | |
| userTokenData = existingTokenData.get(); | |
| userTokenData.setToken(userToken.getToken()); | |
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | |
| } else { | |
| // User token nahi mila => naya insert karna hai | |
| userTokenData = new UserTokenData(); | |
| userTokenData.setUserId(userToken.getUserId()); | |
| userTokenData.setToken(userToken.getToken()); | |
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | |
| } | |
| userTokenRepo.save(userTokenData); | |
| return "Save Successfully"; | |
| } | |
| public String updateToken(UserToken userToken) { | |
| if (userToken == null || userToken.getUserId() == null || userToken.getToken() == null) { | |
| logger.error("Invalid user token data provided"); | |
| return "Error: Invalid user token data"; | |
| } | |
| try { | |
| Optional<UserTokenData> existingTokenData = userTokenRepo.findById(userToken.getUserId()); | |
| UserTokenData userTokenData; | |
| if (existingTokenData.isPresent()) { | |
| // User token exist karta hai => update karna hai | |
| userTokenData = existingTokenData.get(); | |
| userTokenData.setToken(userToken.getToken()); | |
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | |
| } else { | |
| // User token nahi mila => naya insert karna hai | |
| userTokenData = new UserTokenData(); | |
| userTokenData.setUserId(userToken.getUserId()); | |
| userTokenData.setToken(userToken.getToken()); | |
| userTokenData.setUpdatedAt(new Timestamp(System.currentTimeMillis())); | |
| } | |
| userTokenRepo.save(userTokenData); | |
| logger.info("Token updated successfully for user: {}", userToken.getUserId()); | |
| return "Token saved successfully"; | |
| } catch (Exception e) { | |
| logger.error("Error saving user token for user: {}", userToken.getUserId(), e); | |
| return "Error saving token"; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java
around lines 83 to 103, the updateToken method lacks input validation and error
handling for database operations. Add validation to check if the userToken
parameter and its fields (userId and token) are not null or empty before
proceeding. Wrap the database operations in a try-catch block to handle
potential exceptions, and return appropriate error messages or throw custom
exceptions if validation fails or database errors occur.
| start-sms-scheduler=true | ||
| cron-scheduler-sms=0 0/1 * * * ? * | ||
|
|
||
| # Firebase Configuration |
There was a problem hiding this comment.
Please add similar variables to the _docker file
There was a problem hiding this comment.
There was a problem hiding this comment.
I have added firebase configuration in _ docker file
| cors.allowed-origins=${CORS_ALLOWED_ORIGINS} | ||
|
|
||
| # Firebase Configuration | ||
| firebase.enabled=@env.FIREBASE_ENABLE@ |
|
@SauravBizbRolly please look into the security hotspot? |
|
@drtechie |
|
📋 Description
JIRA ID:
3.3.0 release
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation