Standardize API error handling and JSON parsing across major controllers#190
Standardize API error handling and JSON parsing across major controllers#190ManikaKutiyal wants to merge 1 commit intoPSMRI:mainfrom
Conversation
📝 WalkthroughWalkthroughThis pull request refactors multiple REST controller endpoints across the healthcare application to return Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java (1)
371-393:⚠️ Potential issue | 🟠 MajorInconsistent: Method not refactored to match standardized pattern.
This method still returns
Stringand uses a localtry-catchblock, while all other methods in this controller have been refactored to returnResponseEntity<String>and declarethrows Exceptionfor theGlobalExceptionHandlerto manage. This breaks the PR's objective of standardized error handling.Proposed fix to align with the standardized pattern
`@Operation`(summary = "Get beneficiary medication history") `@PostMapping`(value = { "/getBenMedicationHistory" }) - public String getBenMedicationHistory(`@Param`(value = "{\"benRegID\":\"Long\"}") `@RequestBody` String comingRequest) { + public ResponseEntity<String> getBenMedicationHistory( + `@Param`(value = "{\"benRegID\":\"Long\"}") `@RequestBody` String comingRequest) throws Exception { OutputResponse response = new OutputResponse(); - logger.info("getBenMedicationHistory request:" + comingRequest); - try { - JSONObject obj = new JSONObject(comingRequest); - if (obj.has("benRegID")) { - Long benRegID = obj.getLong("benRegID"); - String s = commonServiceImpl.getMedicationHistoryData(benRegID); - response.setResponse(s); - } else { - logger.info("Invalid Request Data."); - response.setError(5000, "Invalid request"); - } - logger.info("getBenMedicationHistory response:" + response); - } catch (Exception e) { - response.setError(5000, "Error while getting medication history"); - logger.error("Error in getBenMedicationHistory:" + e); + JSONObject obj = new JSONObject(comingRequest); + if (obj.has("benRegID") && !obj.isNull("benRegID")) { + Long benRegID = obj.getLong("benRegID"); + String s = commonServiceImpl.getMedicationHistoryData(benRegID); + response.setResponse(s); + } else { + logger.info("Invalid Request Data."); + response.setError(5000, "Invalid request"); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); } - return response.toString(); + logger.info("getBenMedicationHistory response:" + response); + return ResponseEntity.ok(response.toString()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java` around lines 371 - 393, The getBenMedicationHistory method currently returns String and handles exceptions locally; update it to follow the controller standard: change the signature to return ResponseEntity<String> and declare throws Exception, remove the local try-catch, let exceptions propagate to the GlobalExceptionHandler, keep the same logic for parsing incoming JSON and calling commonServiceImpl.getMedicationHistoryData(benRegID) and building OutputResponse, and finally return ResponseEntity.ok(response.toString()) (or an appropriate ResponseEntity when the request is invalid) so the GlobalExceptionHandler manages errors consistently.
🟠 Major comments (18)
src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java-233-238 (1)
233-238:⚠️ Potential issue | 🟠 MajorSame issue: "Invalid request" should return HTTP 400, not 500.
Apply the same fix as
quickSearchNewfor consistency.Proposed fix
if (searchList == null) { - response.setError(5000, "Invalid request"); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response.toString()); + response.setError(400, "Invalid request"); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); } else { return ResponseEntity.ok(searchList); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java` around lines 233 - 238, The null-check branch currently treats a missing/invalid searchList as a server error; change it to return a 400 Bad Request like quickSearchNew does: when searchList == null, call response.setError(...) with the same client-error code/message used in quickSearchNew and return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()) instead of INTERNAL_SERVER_ERROR; update the null branch in RegistrarController's method that references searchList/response to mirror quickSearchNew's behavior.src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java-216-221 (1)
216-221:⚠️ Potential issue | 🟠 MajorInconsistent HTTP status code - "Invalid request" should return 400, not 500.
Returning HTTP 500 INTERNAL_SERVER_ERROR for an "Invalid request" is semantically incorrect. A null
searchListindicating invalid input should return HTTP 400 BAD_REQUEST, which is howbenAdvanceSearchNew(line 250) handles the same scenario.Proposed fix
if (searchList == null) { - response.setError(5000, "Invalid request"); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response.toString()); + response.setError(400, "Invalid request"); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); } else { return ResponseEntity.ok(searchList); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java` around lines 216 - 221, The null-check branch that currently treats a null searchList as an internal server error should instead return a 400 Bad Request; update the block in RegistrarController where searchList is checked (the branch that calls response.setError(5000, "Invalid request") and returns ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response.toString())) to use HttpStatus.BAD_REQUEST (and keep or adjust the error payload via response.setError as needed) so it matches the benAdvanceSearchNew behavior and correctly signals client-side invalid input.src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java-392-406 (1)
392-406:⚠️ Potential issue | 🟠 MajorMissing validation for
benGovIdMapUpdateResin success condition.Same issue as
createBeneficiary-benGovIdMapUpdateResis computed at line 392 but not validated at lines 399-400.Proposed fix
- if (benRegID >= 0 && benDemoUpdateRes >= 0 && benPhonMapUpdateRes >= 0 - && benbenDemoOtherUpdateRes >= 0 && benImageUpdateRes >= 0) { + if (benRegID >= 0 && benDemoUpdateRes >= 0 && benPhonMapUpdateRes >= 0 + && benGovIdMapUpdateRes >= 0 && benbenDemoOtherUpdateRes >= 0 && benImageUpdateRes >= 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java` around lines 392 - 406, The success condition is missing validation of benGovIdMapUpdateRes (result of registrarServiceImpl.updateBenGovIdMapping); update the if-condition that currently checks benRegID, benDemoUpdateRes, benPhonMapUpdateRes, benbenDemoOtherUpdateRes, and benImageUpdateRes to also include benGovIdMapUpdateRes >= 0 so the update only succeeds when the government ID mapping update succeeded; adjust the same conditional in RegistrarController (around where benGovIdMapUpdateRes is set) and keep the existing error branch unchanged.src/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.java-72-72 (1)
72-72:⚠️ Potential issue | 🟠 MajorAvoid logging full request payloads.
Raw request-body logging here can expose sensitive patient identifiers. Log a minimal marker (endpoint/correlation ID) or mask fields before logging.
Proposed safer logging
-logger.info("Request object for Lab Test Result saving :" + requestObj); +logger.info("Received request for Lab Test Result saving"); ... -logger.info("Request obj to fetch lab tests :" + requestOBJ); +logger.info("Received request to fetch lab tests");Also applies to: 104-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.java` at line 72, The logger.info call in LabTechnicianController that prints the full request object (e.g., "Request object for Lab Test Result saving :" + requestObj) must be changed to avoid logging raw patient data; in the method that handles saving lab test results replace the full request payload log with a minimal, non-sensitive marker such as the endpoint name and a correlation/request ID (or log a sanitized version of requestObj with PII fields masked), and apply the same change for the other logger.info instance at the corresponding save flow; ensure you reference the existing logger usage in LabTechnicianController and preserve the correlation ID or request-context value you already generate for tracing.src/main/java/com/iemr/hwc/utils/exception/IEMRException.java-17-20 (1)
17-20:⚠️ Potential issue | 🟠 MajorGuard against null
httpStatusin the custom constructor.If
httpStatusis null here, downstream handling (seesrc/main/java/com/iemr/hwc/utils/exception/GlobalExceptionHandler.java, Line [23]) can fail while building the error response.🔧 Proposed fix
public IEMRException(String message, int statusCode, HttpStatus httpStatus) { super(message); this.statusCode = statusCode; - this.httpStatus = httpStatus; + this.httpStatus = (httpStatus != null) ? httpStatus : HttpStatus.INTERNAL_SERVER_ERROR; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/utils/exception/IEMRException.java` around lines 17 - 20, The IEMRException(String message, int statusCode, HttpStatus httpStatus) constructor must guard against a null httpStatus to avoid downstream NPEs (e.g., GlobalExceptionHandler). Update the constructor in class IEMRException to check if the passed httpStatus is null and, if so, assign a safe default (e.g., HttpStatus.INTERNAL_SERVER_ERROR) to this.httpStatus; otherwise assign the provided value, leaving statusCode and message handling unchanged.src/main/java/com/iemr/hwc/utils/exception/GlobalExceptionHandler.java-39-46 (1)
39-46:⚠️ Potential issue | 🟠 MajorGeneric handler is coupling HTTP status to internal error codes and exposing raw exception text.
response.setError(ex)usesOutputResponse.setError(Throwable)fromsrc/main/java/com/iemr/hwc/utils/response/OutputResponse.java(Line [89]-Line [144]), which serializesthrown.getMessage()and assigns internal app codes (Line [44]-Line [55]).
Then Line [42]-Line [45] attempts HTTP mapping from those internal codes, which is unreliable (and currently inconsistent withBAD_REQUESTconstant value in the same class).🔧 Proposed fix
import org.springframework.web.context.request.WebRequest; +import org.springframework.web.server.ResponseStatusException; @@ public ResponseEntity<OutputResponse> handleGlobalException(Exception ex, WebRequest request) { logger.error("Unhandled Exception: ", ex); OutputResponse response = new OutputResponse(); - response.setError(ex); + response.setError(OutputResponse.GENERIC_FAILURE, + "An unexpected error occurred. Please try again later.", "FAILURE"); HttpStatus status = HttpStatus.INTERNAL_SERVER_ERROR; - if (response.getStatusCode() == 404) { - status = HttpStatus.NOT_FOUND; - } else if (response.getStatusCode() == 400) { - status = HttpStatus.BAD_REQUEST; + if (ex instanceof ResponseStatusException) { + ResponseStatusException rse = (ResponseStatusException) ex; + status = HttpStatus.valueOf(rse.getStatusCode().value()); } return new ResponseEntity<>(response, status); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/utils/exception/GlobalExceptionHandler.java` around lines 39 - 46, The handler currently calls response.setError(ex) (using OutputResponse.setError(Throwable)) which serializes thrown.getMessage() and uses internal codes to drive HTTP mapping in GlobalExceptionHandler; instead stop passing raw Throwable to OutputResponse and map HTTP status from explicit exception types or from response.getStatusCode() reliably, and ensure the BAD_REQUEST constant value is used for comparison. Change GlobalExceptionHandler.handle... to create a sanitized error payload (no raw exception message) and call a new OutputResponse.setError(String code, String userMessage) or setErrorCode/setErrorMessage methods rather than setError(Throwable); adjust OutputResponse.setError(Throwable) (or add an overload) to not expose thrown.getMessage() and to populate only internal error codes, and update the status-mapping logic in GlobalExceptionHandler to use exception instanceof checks (e.g., IllegalArgumentException -> BAD_REQUEST) or a canonical statusCode field rather than comparing to the leaked internal constants. Ensure unique symbols referenced: GlobalExceptionHandler, OutputResponse.setError(Throwable), response.getStatusCode(), and BAD_REQUEST constant are updated accordingly.src/main/java/com/iemr/hwc/controller/covid19/CovidController.java-87-88 (1)
87-88:⚠️ Potential issue | 🟠 MajorUse
setError(...)insaveBenCovidDoctorDataerror branches.Current 400/500 branches use
setResponse(...), which makes failure payloads inconsistent with standardized error responses.Also applies to: 95-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/covid19/CovidController.java` around lines 87 - 88, In saveBenCovidDoctorData replace uses of response.setResponse(...) in the error branches (both the 400 and 500 handlers where response.setResponse("Invalid request") and similar are used) with response.setError(...) so the failure payload matches the standardized error shape; keep the same ResponseEntity.status(...) returns and body(response.toString()) but populate the error message via setError(...) (also update the other occurrence in the method that currently uses setResponse(...) to use setError(...)).src/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.java-78-79 (1)
78-79:⚠️ Potential issue | 🟠 MajorAvoid logging full request payloads at INFO in clinical flows.
This logs full request content, which can expose beneficiary PHI/PII in logs. Prefer logging request identifiers/correlation IDs only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.java` around lines 78 - 79, The current logger.info call logs the entire requestObj which may contain PHI/PII; replace it with a minimal log that records only non-sensitive identifiers (e.g., correlation/request id or beneficiary ID) instead of the full payload. Update the logging around the call to cSServiceImpl.saveCancerScreeningNurseData(jsnOBJ, Authorization) to reference only safe identifiers extracted from requestObj (e.g., requestObj.getRequestId() or requestObj.getBeneficiaryId()) and remove or redact any full payload prints; ensure Authorization or other sensitive headers are not logged either.src/main/java/com/iemr/hwc/controller/adolescent/ChildhoodAdolescenceController.java-87-94 (1)
87-94:⚠️ Potential issue | 🟠 Major
saveDoctorDataCACmisses non-success handling fori != 1.If service returns non-success, this path still returns 200 without a proper error response. Add explicit failure branch with 5xx (or appropriate status).
Suggested fix
if (jsnOBJ != null) { int i = adolescentAndChildCareService.saveDoctorDataCAC(jsnOBJ, Authorization); - if (i == 1) + if (i == 1) { response.setResponse("Data saved successfully"); + } else { + response.setError(5000, "Unable to save data"); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response.toString()); + } } else { response.setError(5000, "Invalid request object : NULL"); return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/adolescent/ChildhoodAdolescenceController.java` around lines 87 - 94, The controller currently treats any non-null request as success even when adolescentAndChildCareService.saveDoctorDataCAC(jsnOBJ, Authorization) returns i != 1; add an explicit failure branch after the call to saveDoctorDataCAC to set an error on the response (e.g., response.setError(<code>, "Failed to save doctor data")) and return a non-200 ResponseEntity (use ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response.toString()) or a more appropriate 4xx/5xx) so the method does not fall through to ResponseEntity.ok when i != 1; locate the check around saveDoctorDataCAC and update that conditional flow accordingly.src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java-82-83 (1)
82-83:⚠️ Potential issue | 🟠 MajorFailure branches should use
setError(...), notsetResponse(...).These branches return 4xx/5xx but still populate response as a normal response payload message. Use
setErrorto keep error structure consistent.Also applies to: 105-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java` around lines 82 - 83, In GeneralOPDController replace uses of response.setResponse(...) with response.setError(...) in all failure branches (e.g., the branch that currently does response.setResponse("Invalid request") and the branches around lines 105-110) so that error responses use the error payload field; ensure you still return the same ResponseEntity status (HttpStatus.BAD_REQUEST/other 4xx/5xx) and call response.toString() (or the same serialization) after setting setError(...) to keep response shape consistent.src/main/java/com/iemr/hwc/controller/ncdCare/NCDCareController.java-66-71 (1)
66-71:⚠️ Potential issue | 🟠 MajorMalformed JSON requests are not handled before parsing, causing unhandled exceptions instead of 400 responses.
JsonParser.parseString(...)throwsJsonSyntaxExceptionon invalid JSON and is not wrapped in try-catch, bypassing your BAD_REQUEST handling. This occurs in both methods:
- Lines 66-71: Only null-checks before parsing; malformed JSON throws uncaught exception
- Lines 92-93: Parses without any validation; null check is unreachable on parse failure
Wrap parsing in try-catch to normalize invalid JSON to 400 BAD_REQUEST responses.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/ncdCare/NCDCareController.java` around lines 66 - 71, The controller currently calls JsonParser.parseString(...) directly in saveBenNCDCareNurseData (and the other method that parses at lines ~92-93), which throws JsonSyntaxException on malformed JSON; wrap the parsing logic in a try-catch that catches com.google.gson.JsonSyntaxException (and optionally IllegalStateException), and on catch return a ResponseEntity with HttpStatus.BAD_REQUEST and an OutputResponse indicating invalid JSON; update both saveBenNCDCareNurseData and the other parsing method to perform this guarded parse (using JsonParser.parseString) before accessing jsnOBJ so malformed requests produce a 400 instead of an uncaught exception.src/main/java/com/iemr/hwc/controller/ncdCare/NCDCareController.java-98-103 (1)
98-103:⚠️ Potential issue | 🟠 MajorUse
setError(...)in non-2xx paths to keep the standardized error contract.Error branches currently call
setResponse(...). This makes failure payloads inconsistent with the centralized error shape expected by clients.Suggested fix
- response.setResponse("Unable to save data"); + response.setError(5000, "Unable to save data"); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response.toString()); } } else { - response.setResponse("Invalid request"); + response.setError(5000, "Invalid request"); return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/ncdCare/NCDCareController.java` around lines 98 - 103, Replace usage of response.setResponse(...) with response.setError(...) in the non-2xx branches of NCDCareController so the failure payloads match the standardized error contract: specifically update the branches that currently call response.setResponse("Unable to save data") and response.setResponse("Invalid request") to call response.setError(...) before returning ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)... and ResponseEntity.status(HttpStatus.BAD_REQUEST)... respectively, ensuring the same response object (response) and return statements remain unchanged.src/main/java/com/iemr/hwc/controller/adolescent/ChildhoodAdolescenceController.java-67-74 (1)
67-74:⚠️ Potential issue | 🟠 MajorUse
setError()for invalid request and validate blank payloads before parsing.The null check does not protect against empty strings;
JsonParser.parseString("")throws an exception. UsesetError(int code, String message)instead ofsetResponse()to match the error handling pattern used elsewhere in this codebase (see line 91:response.setError(5000, "Invalid request object : NULL")).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/adolescent/ChildhoodAdolescenceController.java` around lines 67 - 74, The null check for requestObj is insufficient because empty strings will cause JsonParser.parseString(requestObj) to throw; before parsing, validate that requestObj is not null and not blank (e.g., trim().isEmpty()), and when invalid call response.setError(5000, "Invalid request object : NULL or blank") (rather than response.setResponse) to match existing error handling; keep the existing flow that calls JsonParser.parseString(requestObj).getAsJsonObject(), logs the request, and then invokes adolescentAndChildCareService.saveNurseData(jsnOBJ, Authorization) only when the payload is non-blank.src/main/java/com/iemr/hwc/controller/anc/AntenatalCareController.java-74-76 (1)
74-76:⚠️ Potential issue | 🟠 MajorAdd try-catch to handle malformed JSON from JsonParser.parseString().
Invalid or blank JSON input will throw
JsonSyntaxExceptionbefore reaching the error handler, causing a 5xx response instead of a proper 4xx error. Wrap theparseString().getAsJsonObject()call in try-catch to handleJsonSyntaxExceptionand returnBAD_REQUESTstatus with an appropriate error message.Note: This pattern affects multiple endpoints in this controller (
saveBenANCDoctorData,updateANCCareNurse,updateANCHistoryNurse,updateANCVitalNurse,updateANCExaminationNurse,updateANCDoctorData, etc.), all with the same vulnerability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/anc/AntenatalCareController.java` around lines 74 - 76, Wrap the JsonParser.parseString(...).getAsJsonObject() call in AntenatalCareController inside a try-catch that catches com.google.gson.JsonSyntaxException, so malformed or blank JSON triggers a handled path; in the catch return a ResponseEntity with HttpStatus.BAD_REQUEST and a clear error message (instead of allowing a 5xx), and apply this same change to the other controller methods that parse JSON (e.g., saveBenANCDoctorData, updateANCCareNurse, updateANCHistoryNurse, updateANCVitalNurse, updateANCExaminationNurse, updateANCDoctorData) to ensure all JSON parsing is guarded and returns 400 on bad input.src/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.java-73-79 (1)
73-79:⚠️ Potential issue | 🟠 MajorParse happens before robust request validation, so malformed JSON can escape 400 handling.
This method parses first; invalid/blank JSON throws before your explicit BAD_REQUEST response path. Add null/blank checks and map parse failures to 400.
Suggested pattern
OutputResponse response = new OutputResponse(); - if (null != requestObj) { - JsonObject jsnOBJ = JsonParser.parseString(requestObj).getAsJsonObject(); + if (requestObj == null || requestObj.isBlank()) { + response.setError(5000, "Invalid request"); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); + } + JsonObject jsnOBJ; + try { + jsnOBJ = JsonParser.parseString(requestObj).getAsJsonObject(); + } catch (RuntimeException ex) { + response.setError(5000, "Invalid request"); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); + } logger.info("Request object for CS nurse data saving :" + requestObj); String nurseDataSaveSuccessFlag = cSServiceImpl.saveCancerScreeningNurseData(jsnOBJ, Authorization); response.setResponse(nurseDataSaveSuccessFlag); - } else { - response.setError(5000, "Invalid request"); - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); - } return ResponseEntity.ok(response.toString());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.java` around lines 73 - 79, The method saveBenCancerScreeningNurseData currently calls JsonParser.parseString(requestObj) before validating the payload, which lets malformed or blank JSON throw an exception instead of returning 400; add a null/blank check for requestObj at the start of saveBenCancerScreeningNurseData and wrap JsonParser.parseString(...) in a try-catch that catches JSON parsing exceptions (e.g., JsonSyntaxException) and returns a BAD_REQUEST ResponseEntity with an appropriate OutputResponse error message; keep the call to cSServiceImpl.saveCancerScreeningNurseData(jsnOBJ, Authorization) only after successful validation/parse.src/main/java/com/iemr/hwc/controller/pnc/PostnatalCareController.java-70-75 (1)
70-75:⚠️ Potential issue | 🟠 MajorAdd input validation before JSON parsing to handle malformed/blank requests with 400 BAD_REQUEST.
Lines 96 and 125 parse JSON without prior guards, allowing malformed input to throw unhandled exceptions. Line 74 checks null but not empty strings. Wrap parsing in try-catch blocks and return HTTP 400 for parse failures, not 500.
Affected methods
- Line 96 (saveBenPNCDoctorData):
JsonParser.parseString(requestObj)— no guard- Line 125 (getBenVisitDetailsFrmNursePNC):
new JSONObject(comingRequest)— no guard- Line 74 (saveBenPNCNurseData): checks null, but not empty/malformed
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/pnc/PostnatalCareController.java` around lines 70 - 75, Add input validation and JSON-parse error handling for saveBenPNCNurseData, saveBenPNCDoctorData, and getBenVisitDetailsFrmNursePNC: first check that the incoming request string is not null and not blank/empty before parsing, then wrap JsonParser.parseString(...) and new JSONObject(...) calls in try-catch blocks that catch JSON parsing exceptions and return a ResponseEntity with HTTP 400 BAD_REQUEST and a clear error message; ensure existing exception paths that used to throw propagate 500 only for unexpected errors and update logger calls to record the parse exception details for debugging.src/main/java/com/iemr/hwc/controller/covid19/CovidController.java-116-118 (1)
116-118:⚠️ Potential issue | 🟠 Major
obj.length() > 1is not sufficient to validate required keys—useobj.has()to prevent unhandled exceptions.
obj.length() > 1only verifies there are multiple keys; it doesn't confirm "benRegID" and "visitCode" exist. If missing,getLong()throwsJSONException, which gets caught by the global exception handler and returns a 500 error instead of the intended 400. Other controllers in this codebase (CancerScreeningController, NeonatalController) already follow the correct pattern.Suggested fix
- if (obj.length() > 1) { + if (obj.has("benRegID") && obj.has("visitCode")) { Long benRegID = obj.getLong("benRegID"); Long visitCode = obj.getLong("visitCode");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/covid19/CovidController.java` around lines 116 - 118, The check using obj.length() > 1 in CovidController.java is insufficient; change the validation to explicitly verify presence of required keys using obj.has("benRegID") && obj.has("visitCode") before calling obj.getLong("benRegID") and obj.getLong("visitCode"), and if either key is missing return a 400 Bad Request with a clear message instead of allowing a JSONException to bubble up; update the block around obj.getLong(...) to first use obj.has(...) for both "benRegID" and "visitCode" and handle the missing-key case consistently with CancerScreeningController/NeonatalController.src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java-73-79 (1)
73-79:⚠️ Potential issue | 🟠 MajorMove JSON parsing after validation to prevent uncaught exceptions.
JsonParser.parseString(requestObj).getAsJsonObject()throwsJsonSyntaxExceptionon blank or malformed JSON, bypassing error handling and causing a 500 response instead of 400 BAD_REQUEST.In the first method (lines 73-79), the null check is insufficient; in the second method (lines 99-100), JSON parsing occurs without any pre-validation, making the subsequent null check on
jsnOBJunreachable.Wrap parsing in a try-catch or validate input format before parsing.
Also applies to: 99-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java` around lines 73 - 79, The code currently calls JsonParser.parseString(requestObj).getAsJsonObject() (seen in saveBenGenOPDNurseData) before validating requestObj, which throws JsonSyntaxException for blank/malformed JSON; move JSON parsing to after a null/blank check on requestObj, wrap parsing in a try-catch for JsonSyntaxException, and on catch return a ResponseEntity with HttpStatus.BAD_REQUEST (include a helpful message); apply the same fix to the other method that parses JSON before validation (any place using JsonParser.parseString), and ensure you do not call generalOPDServiceImpl.saveNurseData with an unparsed or null jsnOBJ.
🟡 Minor comments (2)
src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java-204-204 (1)
204-204:⚠️ Potential issue | 🟡 MinorIncorrect log message - copy-paste error.
The log message references
getBeneficiaryDetailsbut this is thegetBeneficiaryImagemethod.Proposed fix
- logger.info("getBeneficiaryDetails response :" + response); + logger.info("getBeneficiaryImage response :" + response);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java` at line 204, In RegistrarController, update the incorrect log in the getBeneficiaryImage method: change the logger.info call that currently says "getBeneficiaryDetails response :" + response to reference getBeneficiaryImage instead so the log message accurately identifies the method (modify the logger.info in getBeneficiaryImage to a descriptive message like "getBeneficiaryImage response : " + response).src/main/java/com/iemr/hwc/controller/common/master/CommonMasterController.java-119-121 (1)
119-121:⚠️ Potential issue | 🟡 MinorReplace null check with domain validation for both required path variables.
Both
currentImmunizationServiceIDandvisitCategoryIDare required path variables (Spring default), so null checks are unreachable. Instead, validate that IDs are domain-valid (e.g.,<= 0), which catches both malformed input and semantically invalid values like negative IDs.Suggested fix
- if (currentImmunizationServiceID == null) { + if (currentImmunizationServiceID <= 0 || visitCategoryID <= 0) { response.setError(5000, "invalid request / NULL"); return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/common/master/CommonMasterController.java` around lines 119 - 121, The null check for currentImmunizationServiceID is unreachable; instead validate both path variables for domain validity by checking currentImmunizationServiceID and visitCategoryID for non-positive values (<= 0) and treat those as invalid input. In the controller method that uses currentImmunizationServiceID and visitCategoryID, replace the null checks with conditions like if (currentImmunizationServiceID <= 0 || visitCategoryID <= 0) then call response.setError(5000, "invalid request / NULL or invalid id") and return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); ensure you reference the existing symbols currentImmunizationServiceID, visitCategoryID, response and the ResponseEntity return path so the logic is applied to both required path variables.
🧹 Nitpick comments (13)
src/main/java/com/iemr/hwc/utils/http/HTTPRequestInterceptor.java (2)
111-114: Avoid redundanttoString()calls and fix response ordering.
output.toString()is called twice on lines 112 and 114. Additionally,setContentTypeandsetContentLengthshould ideally be set before writing to the output stream to ensure headers are properly committed.♻️ Proposed fix
OutputResponse output = new OutputResponse(); output.setError(e); + String responseBody = output.toString(); response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); - response.getOutputStream().print(output.toString()); response.setContentType(MediaType.APPLICATION_JSON); - response.setContentLength(output.toString().length()); + response.setContentLength(responseBody.length()); + response.getOutputStream().print(responseBody);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/utils/http/HTTPRequestInterceptor.java` around lines 111 - 114, In HTTPRequestInterceptor, avoid calling output.toString() twice and set headers before writing: compute a single String body = output.toString(), call response.setStatus(HttpServletResponse.SC_UNAUTHORIZED), then set response.setContentType(MediaType.APPLICATION_JSON) and response.setContentLength(body.getBytes(StandardCharsets.UTF_8).length) (or setContentLengthLong), and only after that write the body to response.getOutputStream().print(body); this ensures headers are set once and before the response body is committed.
163-176: Consider using defensive escaping for CORS origin patterns to handle all regex metacharacters.The regex conversion only escapes
.and replaces*with.*. While current configuration examples in the codebase (e.g.,http://localhost:*,http://localhost:9090) don't contain other regex metacharacters, usingPattern.quote()provides better defensive coverage. If configuration ever includes patterns with characters like+,?,[, or], the current approach could causePatternSyntaxExceptionor unexpected matching behavior.Consider refactoring to safely escape all regex literals while preserving
*as a wildcard:String[] parts = pattern.split("\\*", -1); String regex = Arrays.stream(parts) .map(java.util.regex.Pattern::quote) .collect(java.util.stream.Collectors.joining(".*"));Alternatively, document the configuration constraint that patterns must contain only alphanumeric characters, dots, colons, and asterisks (no other regex metacharacters).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/utils/http/HTTPRequestInterceptor.java` around lines 163 - 176, The isOriginAllowed method currently only escapes '.' and replaces '*' which can mis-handle other regex metacharacters; update isOriginAllowed to build the regex by splitting each pattern on '*' and using java.util.regex.Pattern.quote for each literal segment then joining with ".*" so '*' remains a wildcard while all other characters are safely escaped (operate on allowedOrigins -> pattern processing inside isOriginAllowed).src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java (1)
217-218: Consider standardizing internal error codes.The response error codes are inconsistent across the controller:
0,500, and5000are used interchangeably. While these are application-level codes (separate from HTTP status codes), standardizing them would improve API consistency and make client-side error handling more predictable.Also applies to: 234-235, 249-250, 271-272, 275-276, 314-315, 339-340, 343-344, 405-406, 409-410, 427-428, 429-430, 448-449
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java` around lines 217 - 218, Standardize the internal application error code used by response.setError in RegistrarController: pick a single internal error code (e.g., 5000 → 500 or vice versa) and replace all inconsistent usages of response.setError(...) that accompany ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) so they use that single chosen code; update every occurrence where the local variable response is used with setError (including the grouped spots noted around lines with ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) and the ranges mentioned) to the chosen code so all internal-server-error responses emitted by RegistrarController.call the same application-level error code while leaving the HTTP status unchanged.src/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.java (1)
74-179: Extract shared JSON parse/validate flow to a helper.The same parse/validate/error-response pattern is repeated across methods. A small private helper will reduce boilerplate and keep behavior consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.java` around lines 74 - 179, Extract the repeated JSON parsing/validation into a private helper (e.g., private JsonObject parseAndRequireFields(String requestBody, String... requiredFields)) that uses JsonParser.parseString(requestBody).getAsJsonObject(), checks for null/isJsonNull and each required field with has(...), and throws or returns a standardized error ResponseEntity when validation fails; then replace the duplicated code in getBeneficiaryPrescribedProcedure, getLabResultForVisitCode, getProcedureComponentMappedMasterData (and the saveLabTestResult handler block) to call parseAndRequireFields, use the returned JsonObject, and remove the duplicated JsonParser/validation/error-response logic so all handlers call the single helper for consistent behavior.src/main/java/com/iemr/hwc/utils/exception/IEMRException.java (1)
13-14: Replace duplicated default status code magic number.
5000is repeated in multiple constructors; extracting a single constant avoids drift.♻️ Proposed refactor
public class IEMRException extends RuntimeException { + private static final int DEFAULT_STATUS_CODE = 5000; private final int statusCode; private final HttpStatus httpStatus; @@ - this.statusCode = 5000; + this.statusCode = DEFAULT_STATUS_CODE; this.httpStatus = HttpStatus.INTERNAL_SERVER_ERROR; @@ - this.statusCode = 5000; + this.statusCode = DEFAULT_STATUS_CODE; this.httpStatus = HttpStatus.INTERNAL_SERVER_ERROR; } }Also applies to: 25-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/utils/exception/IEMRException.java` around lines 13 - 14, Replace the duplicated magic number 5000 in IEMRException by adding a single private static final int DEFAULT_STATUS_CODE = 5000 and use that constant wherever statusCode is initialized in the constructors (e.g., in the IEMRException constructors that currently set this.statusCode = 5000 and this.httpStatus = HttpStatus.INTERNAL_SERVER_ERROR); update any other constructors that set 5000 (lines referenced around constructors) to reference DEFAULT_STATUS_CODE, keeping httpStatus initialization as HttpStatus.INTERNAL_SERVER_ERROR (or extract DEFAULT_HTTP_STATUS if you prefer consistency).src/main/java/com/iemr/hwc/controller/anc/AntenatalCareController.java (1)
60-61: Use a singleANCServicedependency field.Having both
ancServiceandaNCServicein the same controller is unnecessary and makes ownership/call-paths harder to follow.Suggested cleanup
- `@Autowired` - private ANCService aNCService; @@ - SysObstetricExamination sysObstetricExamination = aNCService + SysObstetricExamination sysObstetricExamination = ancService .getHRPInformation(commonUtilityClass.getBeneficiaryRegId());Also applies to: 293-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/anc/AntenatalCareController.java` around lines 60 - 61, There are duplicate ANCService fields (ancService and aNCService) in AntenatalCareController; remove the redundant field (keep a single, consistently named ANCService field, e.g., ancService) and update all references/calls in the class to use that single field (search for usages of both ancService and aNCService and replace the latter with the retained field), and ensure the `@Autowired` annotation remains on the single field so dependency injection still works.src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java (7)
786-795: Consider adding input validation for consistency.Unlike other methods in this controller,
getProviderSpecificDatadoes not validate the incoming request body before calling the service. IfcomingRequestis null or malformed, the service layer might throw an unexpected exception. Consider adding a null check consistent with other endpoints in this controller.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java` around lines 786 - 795, getProviderSpecificData lacks input validation for the request body; add a null/empty check for the incomingRequest (parameter comingRequest) before calling commonServiceImpl.getProviderSpecificData, and if invalid return a bad request response (e.g., ResponseEntity.badRequest() with a clear message) consistent with other controller methods; ensure you still log the invalid input and avoid calling commonServiceImpl.getProviderSpecificData when comingRequest is null/blank, updating OutputResponse usage accordingly.
81-91: Missing error handling when service returns null with valid inputs.When
providerServiceMapIDandserviceIDare both valid butcommonDoctorServiceImpl.getDocWorkListNew()returnsnull, the method returns HTTP 200 with an empty response instead of signaling an error. This differs from similar methods likegetNurseWorkListNew(line 122-128) which explicitly handle null service responses as errors.Suggested fix
if (providerServiceMapID != null && serviceID != null) { String s = commonDoctorServiceImpl.getDocWorkListNew(providerServiceMapID, serviceID, vanID); if (s != null) response.setResponse(s); + else { + response.setError(5000, "Error while getting doctor worklist"); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response.toString()); + } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java` around lines 81 - 91, When providerServiceMapID and serviceID are non-null but commonDoctorServiceImpl.getDocWorkListNew(providerServiceMapID, serviceID, vanID) returns null, treat this as an error: detect null after calling getDocWorkListNew, log an error via logger.error including providerServiceMapID and serviceID, call response.setError(...) with an appropriate code/message (matching the pattern used in getNurseWorkListNew), and return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()) instead of falling through to ResponseEntity.ok; update the block around getDocWorkListNew and response.setResponse(...) to enforce this null-check and error path.
101-112: Consistent null handling issue as ingetDocWorkListNew.Same concern: when the service returns
nullwith valid inputs, an HTTP 200 is returned without error information. Apply the same fix pattern as suggested forgetDocWorkListNew.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java` around lines 101 - 112, The method currently treats a null result from commonDoctorServiceImpl.getDocWorkListNewFutureScheduledForTM(...) as a success and returns HTTP 200; change it to handle null like getDocWorkListNew: after calling getDocWorkListNewFutureScheduledForTM(providerServiceMapID, serviceID, vanID) capture the returned String s, and if s is null log an error (including providerServiceMapID and serviceID), set response.setError(...) with an appropriate code/message, and return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()); otherwise set response.setResponse(s) and return ResponseEntity.ok(response.toString()). Ensure you reference commonDoctorServiceImpl.getDocWorkListNewFutureScheduledForTM, the local variable s, and response when making the change.
873-878: Placeholder implementation noted for user validation.The comment indicates user validation is intended but not yet implemented. This could be a security concern if session extension should be restricted to authenticated users.
Would you like me to open an issue to track implementing user validation for this endpoint?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java` around lines 873 - 878, The extendRedisSession method in WorklistController currently extends sessions without authenticating the caller; update extendRedisSession to verify the requesting user before extending the Redis session by checking the application's authentication mechanism (e.g., SecurityContextHolder, a currentUser/getCurrentUserId helper, session token validation, or existing auth utilities used elsewhere in the controller), return a 401/403 response when the caller is not authenticated/authorized, and only perform the session extension and return the OutputResponse on successful validation; reuse existing auth helper methods used elsewhere in WorklistController to locate the correct validation logic and error responses.
625-636: Missing error handling when service returns null with valid inputs.Same pattern as above—apply consistent null-response error handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java` around lines 625 - 636, The code checks providerServiceMapID and userID then calls commonDoctorServiceImpl.getTCSpecialistWorkListNewForTMPatientApp(...) but does not handle the case where that method returns null for otherwise valid inputs; update the block around providerServiceMapID/userID to detect a null result from getTCSpecialistWorkListNewForTMPatientApp, log an error via logger with context (include providerServiceMapID and userID), set response.setError(...) with an appropriate error code/message, and return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()) instead of proceeding to ResponseEntity.ok; keep the existing response.setResponse(s) branch when s is non-null.
604-615: Missing error handling when service returns null with valid inputs.Same pattern as
getDocWorkListNew—whenproviderServiceMapIDanduserIDare valid butgetTCSpecialistWorkListNewForTM()returnsnull, the method returns HTTP 200 without error information.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java` around lines 604 - 615, The current block in WorklistController calls commonDoctorServiceImpl.getTCSpecialistWorkListNewForTM(providerServiceMapID, userID, serviceID) and only sets response when the returned String s is non-null, but does not handle the case where s == null for valid providerServiceMapID and userID; add an explicit null-check after the call to getTCSpecialistWorkListNewForTM (reference symbols: providerServiceMapID, userID, getTCSpecialistWorkListNewForTM, response) and when s is null log an error (including the PSMID and userID), set response.setError(...) with an appropriate error code/message, and return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.toString()) (mirroring the existing invalid-input branch) so the endpoint does not return HTTP 200 with an empty/undefined payload.
647-658: Missing error handling when service returns null with valid inputs.Same pattern—apply consistent null-response error handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java` around lines 647 - 658, The code in WorklistController calls commonDoctorServiceImpl.getTCSpecialistWorkListNewFutureScheduledForTM(providerServiceMapID, userID, serviceID) and only sets response when the returned String s is non-null; add explicit handling for the case where inputs are valid but s == null: detect null after the call, set an appropriate error on response (e.g., response.setError(5001, "No worklist data found" or similar), log the situation including providerServiceMapID/userID and serviceID, and return a ResponseEntity with an appropriate status (BAD_REQUEST or NOT_FOUND) instead of falling through to a successful 200; update the branch that currently only handles s != null and the else that logs invalid inputs to cover this null-return scenario using the same response/logging conventions (reference: providerServiceMapID, userID, serviceID, commonDoctorServiceImpl.getTCSpecialistWorkListNewFutureScheduledForTM, response.setResponse, response.setError).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 697e45f5-48fe-472a-b11c-1a9a7d117c3b
📒 Files selected for processing (14)
src/main/java/com/iemr/hwc/controller/adolescent/ChildhoodAdolescenceController.javasrc/main/java/com/iemr/hwc/controller/anc/AntenatalCareController.javasrc/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.javasrc/main/java/com/iemr/hwc/controller/common/main/WorklistController.javasrc/main/java/com/iemr/hwc/controller/common/master/CommonMasterController.javasrc/main/java/com/iemr/hwc/controller/covid19/CovidController.javasrc/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.javasrc/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.javasrc/main/java/com/iemr/hwc/controller/ncdCare/NCDCareController.javasrc/main/java/com/iemr/hwc/controller/pnc/PostnatalCareController.javasrc/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.javasrc/main/java/com/iemr/hwc/utils/exception/GlobalExceptionHandler.javasrc/main/java/com/iemr/hwc/utils/exception/IEMRException.javasrc/main/java/com/iemr/hwc/utils/http/HTTPRequestInterceptor.java
| @GetMapping(value = { "/getNurseWorklistNew/{providerServiceMapID}/{serviceID}/{vanID}" }) | ||
| public String getNurseWorkListNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| public ResponseEntity<String> getNurseWorkListNew( | ||
| @PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| @PathVariable("vanID") Integer vanID) { |
There was a problem hiding this comment.
Critical: Missing @PathVariable for serviceID will cause runtime errors.
The URL path contains {serviceID} but the method signature does not include a corresponding @PathVariable("serviceID") parameter. Spring MVC will fail to bind this path variable, causing request handling errors.
Proposed fix
`@GetMapping`(value = { "/getNurseWorklistNew/{providerServiceMapID}/{serviceID}/{vanID}" })
public ResponseEntity<String> getNurseWorkListNew(
`@PathVariable`("providerServiceMapID") Integer providerServiceMapID,
+ `@PathVariable`("serviceID") Integer serviceID,
`@PathVariable`("vanID") Integer vanID) {If serviceID is intentionally unused, consider removing it from the URL path to maintain a clean API contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java`
around lines 117 - 120, The getNurseWorkListNew controller mapping includes
{serviceID} in the `@GetMapping` but the method signature for getNurseWorkListNew
is missing a corresponding `@PathVariable` parameter; update the method signature
to accept `@PathVariable`("serviceID") Integer serviceID (or if serviceID is not
needed, remove {serviceID} from the `@GetMapping` URL) so Spring can bind the path
variable correctly and avoid runtime binding errors.
| @GetMapping(value = { "/getNurseWorkListTcCurrentDate/{providerServiceMapID}/{serviceID}/{vanID}" }) | ||
| public String getNurseWorkListTcCurrentDateNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| public ResponseEntity<String> getNurseWorkListTcCurrentDateNew( | ||
| @PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| @PathVariable("vanID") Integer vanID) { |
There was a problem hiding this comment.
Critical: Missing @PathVariable for serviceID.
Same issue as getNurseWorkListNew—the path includes {serviceID} but the method signature lacks the corresponding parameter binding.
Proposed fix
`@GetMapping`(value = { "/getNurseWorkListTcCurrentDate/{providerServiceMapID}/{serviceID}/{vanID}" })
public ResponseEntity<String> getNurseWorkListTcCurrentDateNew(
`@PathVariable`("providerServiceMapID") Integer providerServiceMapID,
+ `@PathVariable`("serviceID") Integer serviceID,
`@PathVariable`("vanID") Integer vanID) {📝 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.
| @GetMapping(value = { "/getNurseWorkListTcCurrentDate/{providerServiceMapID}/{serviceID}/{vanID}" }) | |
| public String getNurseWorkListTcCurrentDateNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | |
| public ResponseEntity<String> getNurseWorkListTcCurrentDateNew( | |
| @PathVariable("providerServiceMapID") Integer providerServiceMapID, | |
| @PathVariable("vanID") Integer vanID) { | |
| `@GetMapping`(value = { "/getNurseWorkListTcCurrentDate/{providerServiceMapID}/{serviceID}/{vanID}" }) | |
| public ResponseEntity<String> getNurseWorkListTcCurrentDateNew( | |
| `@PathVariable`("providerServiceMapID") Integer providerServiceMapID, | |
| `@PathVariable`("serviceID") Integer serviceID, | |
| `@PathVariable`("vanID") Integer vanID) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java`
around lines 134 - 137, The mapping for getNurseWorkListTcCurrentDateNew
includes {serviceID} but the method signature is missing its `@PathVariable`;
update the getNurseWorkListTcCurrentDateNew method to accept a serviceID
parameter annotated with `@PathVariable` (e.g., Integer serviceID) so the path
variable name matches the mapping and the method signature (ensure the parameter
name matches "serviceID" or specify it in the annotation).
| @GetMapping(value = { "/getNurseWorkListTcFutureDate/{providerServiceMapID}/{serviceID}/{vanID}" }) | ||
| public String getNurseWorkListTcFutureDateNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| public ResponseEntity<String> getNurseWorkListTcFutureDateNew( | ||
| @PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| @PathVariable("vanID") Integer vanID) { |
There was a problem hiding this comment.
Critical: Missing @PathVariable for serviceID.
Same pattern—{serviceID} in path but no binding in method signature.
Proposed fix
`@GetMapping`(value = { "/getNurseWorkListTcFutureDate/{providerServiceMapID}/{serviceID}/{vanID}" })
public ResponseEntity<String> getNurseWorkListTcFutureDateNew(
`@PathVariable`("providerServiceMapID") Integer providerServiceMapID,
+ `@PathVariable`("serviceID") Integer serviceID,
`@PathVariable`("vanID") Integer vanID) {📝 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.
| @GetMapping(value = { "/getNurseWorkListTcFutureDate/{providerServiceMapID}/{serviceID}/{vanID}" }) | |
| public String getNurseWorkListTcFutureDateNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | |
| public ResponseEntity<String> getNurseWorkListTcFutureDateNew( | |
| @PathVariable("providerServiceMapID") Integer providerServiceMapID, | |
| @PathVariable("vanID") Integer vanID) { | |
| `@GetMapping`(value = { "/getNurseWorkListTcFutureDate/{providerServiceMapID}/{serviceID}/{vanID}" }) | |
| public ResponseEntity<String> getNurseWorkListTcFutureDateNew( | |
| `@PathVariable`("providerServiceMapID") Integer providerServiceMapID, | |
| `@PathVariable`("serviceID") Integer serviceID, | |
| `@PathVariable`("vanID") Integer vanID) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java`
around lines 151 - 154, The mapping in WorklistController's
getNurseWorkListTcFutureDateNew has a path variable {serviceID} but the method
signature omits a corresponding `@PathVariable` parameter; update
getNurseWorkListTcFutureDateNew to accept a serviceID parameter annotated with
`@PathVariable` (Integer serviceID) so the URI template variable binds correctly,
keeping the existing providerServiceMapID and vanID parameters intact.
| @GetMapping(value = { "/getLabWorklistNew/{providerServiceMapID}/{serviceID}/{vanID}" }) | ||
| public String getLabWorkListNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| public ResponseEntity<String> getLabWorkListNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| @PathVariable("vanID") Integer vanID) { |
There was a problem hiding this comment.
Critical: Missing @PathVariable for serviceID.
The path /getLabWorklistNew/{providerServiceMapID}/{serviceID}/{vanID} includes {serviceID}, but it is not declared in the method parameters.
Proposed fix
`@GetMapping`(value = { "/getLabWorklistNew/{providerServiceMapID}/{serviceID}/{vanID}" })
public ResponseEntity<String> getLabWorkListNew(`@PathVariable`("providerServiceMapID") Integer providerServiceMapID,
+ `@PathVariable`("serviceID") Integer serviceID,
`@PathVariable`("vanID") Integer vanID) {📝 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.
| @GetMapping(value = { "/getLabWorklistNew/{providerServiceMapID}/{serviceID}/{vanID}" }) | |
| public String getLabWorkListNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | |
| public ResponseEntity<String> getLabWorkListNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | |
| @PathVariable("vanID") Integer vanID) { | |
| `@GetMapping`(value = { "/getLabWorklistNew/{providerServiceMapID}/{serviceID}/{vanID}" }) | |
| public ResponseEntity<String> getLabWorkListNew(`@PathVariable`("providerServiceMapID") Integer providerServiceMapID, | |
| `@PathVariable`("serviceID") Integer serviceID, | |
| `@PathVariable`("vanID") Integer vanID) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java`
around lines 190 - 192, The GetMapping for getLabWorklistNew includes a
{serviceID} path variable but the controller method getLabWorkListNew lacks a
corresponding parameter; update the method signature of getLabWorkListNew to
declare a `@PathVariable`("serviceID") Integer serviceID (keep types and
annotation style consistent with providerServiceMapID and vanID), and ensure any
use of serviceID inside the method (or downstream calls) uses this new
parameter.
| @PostMapping(value = { "/getRadiologist-worklist-New/{providerServiceMapID}/{serviceID}/{vanID}" }) | ||
| public String getRadiologistWorklistNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| public ResponseEntity<String> getRadiologistWorklistNew( | ||
| @PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| @PathVariable("vanID") Integer vanID) { |
There was a problem hiding this comment.
Critical: Missing @PathVariable for serviceID.
Path includes {serviceID} without a corresponding method parameter.
Proposed fix
`@PostMapping`(value = { "/getRadiologist-worklist-New/{providerServiceMapID}/{serviceID}/{vanID}" })
public ResponseEntity<String> getRadiologistWorklistNew(
`@PathVariable`("providerServiceMapID") Integer providerServiceMapID,
+ `@PathVariable`("serviceID") Integer serviceID,
`@PathVariable`("vanID") Integer vanID) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java`
around lines 206 - 209, The getRadiologistWorklistNew method signature is
missing the serviceID `@PathVariable` referenced in the route; update the method
(getRadiologistWorklistNew) to include a parameter annotated with
`@PathVariable`("serviceID") Integer serviceID alongside providerServiceMapID and
vanID so the {serviceID} placeholder is bound correctly, ensuring the
`@PostMapping` path variables and method parameters match exactly.
| @PostMapping(value = { "/getOncologist-worklist-New/{providerServiceMapID}/{serviceID}/{vanID}" }) | ||
| public String getOncologistWorklistNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| public ResponseEntity<String> getOncologistWorklistNew( | ||
| @PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| @PathVariable("vanID") Integer vanID) { |
There was a problem hiding this comment.
Critical: Missing @PathVariable for serviceID.
Path includes {serviceID} without a corresponding method parameter.
Proposed fix
`@PostMapping`(value = { "/getOncologist-worklist-New/{providerServiceMapID}/{serviceID}/{vanID}" })
public ResponseEntity<String> getOncologistWorklistNew(
`@PathVariable`("providerServiceMapID") Integer providerServiceMapID,
+ `@PathVariable`("serviceID") Integer serviceID,
`@PathVariable`("vanID") Integer vanID) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java`
around lines 223 - 226, WorklistController's getOncologistWorklistNew mapping
declares a path variable {serviceID} but the method signature lacks a
corresponding parameter; add a method parameter annotated with `@PathVariable` for
serviceID (e.g., Integer serviceID) to the getOncologistWorklistNew method so
the `@PostMapping` path variables match the method signature and Spring can bind
serviceID correctly.
| @GetMapping(value = { "/getPharma-worklist-New/{providerServiceMapID}/{serviceID}/{vanID}" }) | ||
| public String getPharmaWorklistNew(@PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| public ResponseEntity<String> getPharmaWorklistNew( | ||
| @PathVariable("providerServiceMapID") Integer providerServiceMapID, | ||
| @PathVariable("vanID") Integer vanID) { |
There was a problem hiding this comment.
Critical: Missing @PathVariable for serviceID.
Path includes {serviceID} without a corresponding method parameter.
Proposed fix
`@GetMapping`(value = { "/getPharma-worklist-New/{providerServiceMapID}/{serviceID}/{vanID}" })
public ResponseEntity<String> getPharmaWorklistNew(
`@PathVariable`("providerServiceMapID") Integer providerServiceMapID,
+ `@PathVariable`("serviceID") Integer serviceID,
`@PathVariable`("vanID") Integer vanID) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java`
around lines 240 - 243, The GetMapping path for getPharmaWorklistNew declares
{serviceID} but the method signature lacks a corresponding parameter; update the
method signature for getPharmaWorklistNew to accept a serviceID parameter
annotated with `@PathVariable`("serviceID") (e.g., add `@PathVariable`("serviceID")
Integer serviceID) so the path variable is bound, or alternatively remove
{serviceID} from the `@GetMapping` if it is not needed.
| if (i > 0) { | ||
| if (i == 1) | ||
| response.setResponse("Beneficiary moved to nurse worklist"); | ||
| else | ||
| response.setError(5000, "Beneficiary already present in nurse worklist"); | ||
| } else { | ||
| response.setError(5000, "Error while moving beneficiary to nurse worklist"); | ||
| return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response.toString()); | ||
| } | ||
| return response.toString(); | ||
| return ResponseEntity.ok(response.toString()); |
There was a problem hiding this comment.
Error condition incorrectly returns HTTP 200 OK.
When i > 1 (beneficiary already in worklist), response.setError() is called at line 427, but the method continues to line 432 and returns ResponseEntity.ok(). This returns an error response body with HTTP 200 status, violating REST semantics.
Proposed fix
if (i > 0) {
if (i == 1)
response.setResponse("Beneficiary moved to nurse worklist");
- else
+ else {
response.setError(5000, "Beneficiary already present in nurse worklist");
+ return ResponseEntity.status(HttpStatus.CONFLICT).body(response.toString());
+ }
} else {
response.setError(5000, "Error while moving beneficiary to nurse worklist");
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response.toString());
}
return ResponseEntity.ok(response.toString());Using HTTP 409 CONFLICT is appropriate since the resource already exists in the target state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java`
around lines 423 - 432, The branch in RegistrarController where i > 0
incorrectly returns HTTP 200 even when response.setError(...) is called (i == 1
vs i > 1); update the i > 1 case to return an appropriate error status instead
of falling through to ResponseEntity.ok. Concretely, after calling
response.setError(5000, "Beneficiary already present in nurse worklist") (the
branch that checks i), immediately return
ResponseEntity.status(HttpStatus.CONFLICT).body(response.toString()); so that
the error path using response.setError(...) returns HTTP 409 rather than 200.
| if (authorization == null || authorization.isEmpty()) { | ||
| logger.info("Authorization header is null or empty. Skipping HTTPRequestInterceptor."); | ||
| return true; // Allow the request to proceed without validation | ||
| } | ||
| logger.info("Authorization header is null or empty. Skipping HTTPRequestInterceptor."); | ||
| return true; // Allow the request to proceed without validation | ||
| } |
There was a problem hiding this comment.
Critical: Authentication bypass vulnerability.
Returning true when the Authorization header is null or empty allows unauthenticated requests to bypass security checks entirely and proceed to controllers. Previously, such requests would reach the default case and throw an exception ("Authorization key is NULL").
This change breaks authentication for all protected endpoints. Requests without a valid token should be rejected, not allowed through.
🔒 Proposed fix to reject unauthenticated requests
if (authorization == null || authorization.isEmpty()) {
- logger.info("Authorization header is null or empty. Skipping HTTPRequestInterceptor.");
- return true; // Allow the request to proceed without validation
+ logger.warn("Authorization header is null or empty. Rejecting request.");
+ OutputResponse output = new OutputResponse();
+ output.setError(new Exception("Authorization header is missing or empty."));
+ response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+ response.setContentType(MediaType.APPLICATION_JSON);
+ response.getOutputStream().print(output.toString());
+ response.setContentLength(output.toString().length());
+ return false;
}If certain endpoints intentionally require no authentication (e.g., public health checks), consider using a path-based whitelist similar to the swagger-ui handling rather than blanket-allowing missing tokens.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/hwc/utils/http/HTTPRequestInterceptor.java` around
lines 68 - 71, The interceptor currently allows requests with a missing
Authorization header (in HTTPRequestInterceptor), creating an auth bypass;
change the logic in the method that checks "authorization == null ||
authorization.isEmpty()" (the preHandle/intercept method in
HTTPRequestInterceptor) to reject such requests instead of returning true —
either by returning false and setting an appropriate HTTP 401/403 response or by
throwing the existing authorization exception used in the default case
("Authorization key is NULL"); if you need public endpoints, add a path-based
whitelist similar to the swagger-ui handling and only allow missing tokens for
those paths.
1 participant
Overview
This PR implements standardized RESTful error handling across the HWC-API (Issue #115). It transitions several major controllers from returning
200 OKfor error scenarios to using appropriate HTTP status codes (4xx/5xx) and centralizes exception management.Technical Changes
1. Core Infrastructure
@ControllerAdviceto handleExceptionandIEMRException, ensuring a consistent JSON error response format (OutputResponse) with proper HTTP status codes.JsonParser.parse()calls withJsonParser.parseString()across the refactored controllers.2. Standardized Controllers
The following 10+ controllers were refactored to return
ResponseEntity<String>, removing redundant manual error wrapping and implementing proper status codes:AntenatalCareControllerGeneralOPDControllerRegistrarControllerChildhoodAdolescenceControllerWorklistControllerNCDCareControllerLabTechnicianControllerKey Improvements
400 Bad Request,401 Unauthorized, and500 Internal Server Error.Verification
GlobalExceptionHandlercorrectly catches and formats runtime exceptions.Related
This PR is associated with the frontend changes in the
HWC-UIrepository under the same branch name:fix/standardize-error-handling-115.Summary by CodeRabbit