M2 FHIR bundles creation #123
Conversation
Removed unnecessary 'order by 1 desc' clause from queries.
|
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 pull request refactors FHIR bundle creation architecture by migrating from Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CommonServiceImpl
participant BundleCreators as Bundle Creators<br/>(OPConsult/Diagnostic/etc)
participant CareContextRepo
participant ResourceRepo as FHIR Resources<br/>(Practitioner/Patient/etc)
participant Mongo as MongoDB<br/>(AMRIT_ResourceMongo)
Client->>CommonServiceImpl: Process patient resource creation
CommonServiceImpl->>CareContextRepo: Check clinical context<br/>(visit type, lab tests,<br/>prescriptions, vitals, etc)
CareContextRepo-->>CommonServiceImpl: Context availability
alt OP Consult (General OPD)
CommonServiceImpl->>BundleCreators: processOpConsultRecordBundle()
BundleCreators->>ResourceRepo: Fetch Practitioner, Organization,<br/>Patient, Conditions, Encounter
ResourceRepo-->>BundleCreators: FHIR Resources
BundleCreators->>BundleCreators: PopulateOPConsultRecordResourceBundle()<br/>Build Composition + Document Bundle
BundleCreators->>Mongo: Persist AMRIT_ResourceMongo
Mongo-->>BundleCreators: Success
end
alt Diagnostic Report (lab tests present)
CommonServiceImpl->>BundleCreators: processDiagnosticReportRecordBundle()
BundleCreators->>ResourceRepo: Fetch DiagnosticReports,<br/>Observations
ResourceRepo-->>BundleCreators: FHIR Resources
BundleCreators->>BundleCreators: Assemble bundle
BundleCreators->>Mongo: Persist
Mongo-->>BundleCreators: Success
end
alt Prescription (drugs prescribed)
CommonServiceImpl->>BundleCreators: processPrescriptionRecordBundle()
BundleCreators->>ResourceRepo: Fetch MedicationRequest
ResourceRepo-->>BundleCreators: FHIR Resources
BundleCreators->>Mongo: Persist
Mongo-->>BundleCreators: Success
end
alt Wellness (vitals present)
CommonServiceImpl->>BundleCreators: processWellnessRecordBundle()
BundleCreators->>ResourceRepo: Fetch Observations (vitals)
ResourceRepo-->>BundleCreators: FHIR Resources
BundleCreators->>Mongo: Persist
Mongo-->>BundleCreators: Success
end
alt Discharge Summary (unconditional)
CommonServiceImpl->>BundleCreators: processDischargeSummaryRecordBundle()
BundleCreators->>ResourceRepo: Fetch comprehensive resources
ResourceRepo-->>BundleCreators: FHIR Resources
BundleCreators->>Mongo: Persist
Mongo-->>BundleCreators: Success/Failure
end
CommonServiceImpl->>CommonServiceImpl: Update processed flag
CommonServiceImpl-->>Client: Return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major 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/wipro/fhir/controller/test/Test.java (1)
62-65: Avoid using System.out.println for logging.Use the logger instance already defined in this class instead of
System.out.println(e). The logger call on line 63 is appropriate, but line 64 duplicates the error output.Suggested fix
} catch (Exception e) { logger.error("Unexpected error:" , e); - System.out.println(e); }
🟡 Minor comments (14)
src/main/java/com/wipro/fhir/service/resource_model/EncounterResource.java-65-67 (1)
65-67: Remove unused Appointment import; refactoring is complete.All call sites (OPConsultResourceBundleImpl.java and DischargeSummaryResourceBundleImpl.java) have been properly updated to match the new method signature. The Appointment import at line 29 is unused and should be removed.
src/main/java/com/wipro/fhir/service/bundle_creation/BundleValidator.java-52-54 (1)
52-54: Improve error message accuracy.The error message "Issue in validating the bundle" is misleading since the
IOExceptioncan only be thrown byloadPackageFromClasspath(Line 28), not during bundle validation itself.🔎 Proposed fix for clearer error message
} catch (IOException e) { - throw new FHIRException("Issue in validating the bundle - " + e.getMessage()); + throw new FHIRException("Failed to load NPM validation package - " + e.getMessage()); }src/main/java/com/wipro/fhir/service/bundle_creation/BundleValidator.java-48-50 (1)
48-50: Fix output formatting for better readability.The
System.out.printstatement has two issues:
- Missing newline—all validation messages will run together on one line.
- The label "Error - " is misleading since
getSeverity()can return WARNING, INFORMATION, etc.🔎 Proposed fix
- for(SingleValidationMessage next : outCome.getMessages()) { - System.out.print("Error - " + next.getSeverity() + " - " + next.getLocationString() + " - " + next.getMessage()); - } + for(SingleValidationMessage next : outCome.getMessages()) { + System.out.println(next.getSeverity() + " - " + next.getLocationString() + " - " + next.getMessage()); + }pom.xml-218-224 (1)
218-224: Remove duplicatehapi-fhir-structures-r4dependency.This dependency is already declared at lines 204-208 with the same version (8.4.0). The duplicate declaration is redundant and may cause build warnings.
🔎 Proposed fix
- <!-- - https://mvnrepository.com/artifact/ca.uhn.hapi.fhir/hapi-fhir-structures-r4 --> - <dependency> - <groupId>ca.uhn.hapi.fhir</groupId> - <artifactId>hapi-fhir-structures-r4</artifactId> - <version>8.4.0</version> - </dependency> -src/main/environment/common_docker.properties-120-122 (1)
120-122: Inconsistent environment variable pattern for Docker configuration.Other properties in this file use the
${VAR_NAME}pattern for Docker environment variable substitution (e.g.,${DATABASE_URL},${REDIS_HOST}), butsystemUrluses a placeholder value. This will require manual editing and defeats Docker's environment variable injection purpose.🔎 Proposed fix
spring.redis.host=${REDIS_HOST} -systemUrl= <Enter HIP request URL> +systemUrl=${HIP_SYSTEM_URL}pom.xml-241-247 (1)
241-247: Explicit org.hl7.fhir.utilities version overrides HAPI FHIR's transitive dependency.The project explicitly declares
org.hl7.fhir.utilitiesversion 6.5.18, which overrides the transitive version (6.4.0) managed by HAPI FHIR 8.4.0. Additionally, no code in the repository uses this dependency directly. Consider removing the explicit declaration to rely on HAPI FHIR's dependency management, or clarify why a newer version is needed.src/main/environment/common_test.properties-109-109 (1)
109-109: Potential leading space in property value.There's a space after the equals sign:
systemUrl= <Enter.... This space will be included in the property value, which could cause URL construction issues. Other properties in this file don't have this trailing space (e.g.,abdmFacilityId=).Suggested fix
-systemUrl= <Enter HIP request URL> +systemUrl=<Enter HIP request URL>src/main/java/com/wipro/fhir/service/resource_model/MedicalHistoryResource.java-51-51 (1)
51-51: Incorrect resource ID prefix.The ID is set as
"MedicationRequest-"but this creates aMedicationStatementresource. The prefix should match the resource type.Suggested fix
- ms.setId("MedicationRequest-" + index + "/" + commonService.getUUID()); + ms.setId("MedicationStatement-" + index + "/" + commonService.getUUID());src/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java-10-14 (1)
10-14: serialVersionUID declared but Serializable not implemented.
serialVersionUIDis declared but the class doesn't implementSerializable. Compare withMedicalHistoryDataModelin the codebase which correctly implementsSerializable.Suggested fix
@Data @Component -public class PractitionerDataModel { +public class PractitionerDataModel implements java.io.Serializable { private static final long serialVersionUID = 1L;src/main/java/com/wipro/fhir/service/bundle_creation/DiagnosticRecordResourceBundleImpl.java-83-84 (1)
83-84: Replace TODO placeholder in exception message.The exception message
"TODO - exception - later will implement"is not suitable for production code. Provide a meaningful error message that helps diagnose the issue.- throw new FHIRException("TODO - exception - later will implement"); + throw new FHIRException("Failed to create Diagnostic Report bundle - resource data is null");src/main/java/com/wipro/fhir/service/bundle_creation/WellnessRecordResourceBundle.java-23-24 (1)
23-24: Inconsistent exception types compared to other bundle interfaces.This method throws
org.hl7.fhir.exceptions.FHIRException(from the HAPI FHIR library) and the genericException, whereas other bundle interfaces likePrescriptionResourceBundleandDiagnosticRecordResourceBundlethrowcom.wipro.fhir.utils.exception.FHIRException(the project's custom exception).Using the generic
Exceptionis a code smell—consider using specific exception types. Align with the other interfaces for consistency.🔎 Proposed fix for consistent exception handling
- int processWellnessRecordBundle(ResourceRequestHandler resourceRequestHandler, PatientEligibleForResourceCreation p) - throws org.hl7.fhir.exceptions.FHIRException, Exception; + int processWellnessRecordBundle(ResourceRequestHandler resourceRequestHandler, PatientEligibleForResourceCreation p) + throws FHIRException;src/main/java/com/wipro/fhir/service/bundle_creation/DiagnosticRecordResourceBundleImpl.java-129-130 (1)
129-130: Remove the unusedEncounterparameter fromgetDiagnosticReport()method.The
Encounterparameter is declared but never used in the method implementation. ThegenerateDiagnosticReportResource()method does not reference theencounterparameter, andDiagnosticReportobjects are never populated with encounter data. Either remove this dead parameter or implement its intended use by callingdiagnosticReport.setEncounter()on the created reports if encounter context is needed for the bundle.src/main/java/com/wipro/fhir/service/bundle_creation/OPConsultResourceBundleImpl.java-254-255 (1)
254-255: Typo in SNOMED display text.The display text contains a stray backtick character:
"Clinical consultation report\"should be"Clinical consultation report"`.🔎 Proposed fix
composition - .setType(new CodeableConcept(new Coding("http://snomed.info/sct", "371530004", "Clinical consultation report`"))); + .setType(new CodeableConcept(new Coding("http://snomed.info/sct", "371530004", "Clinical consultation report")));src/main/java/com/wipro/fhir/service/bundle_creation/ImmunizationRecordResourceBundleImpl.java-190-195 (1)
190-195: DuplicatesetCustodiancall.Line 190 sets the custodian, and lines 193-195 set it again with the same value. The second call is redundant.
🔎 Proposed fix
composition.addAuthor(new Reference(practitioner.getIdElement().getValue())); composition.setCustodian(new Reference(organization.getIdElement().getValue())); composition.setTitle("Immunization Record"); - - if (organization != null) { - composition.setCustodian(new Reference(organization.getIdElement().getValue())); - }
🧹 Nitpick comments (23)
src/main/java/com/wipro/fhir/service/resource_model/EncounterResource.java (1)
29-29: Remove unused import.The
Appointmentimport is no longer used after removing the parameter from both methods.🔎 Proposed fix
-import org.hl7.fhir.r4.model.Appointment;src/main/java/com/wipro/fhir/service/bundle_creation/BundleValidator.java (1)
44-44: Reminder: Populate the bundle JSON placeholder before use.The empty
bundleJsonstring is a placeholder. Ensure this is populated with actual FHIR bundle JSON before running the validator.Would you like me to suggest a pattern for reading bundle JSON from a file or external source?
src/main/java/com/wipro/fhir/data/resource_model/MedicalHistoryDataModel.java (1)
35-47: Consider adding bounds check to preventArrayIndexOutOfBoundsException.The constructor directly accesses array indices 0-10 without verifying the array length. If an SP returns fewer columns than expected (e.g., due to schema changes or bugs), this will throw an
ArrayIndexOutOfBoundsException.While this pattern matches other data models in the codebase (ImmunizationDataModel, PractitionerDataModel), adding a bounds check would improve robustness.
🔎 Optional defensive check
public MedicalHistoryDataModel(Object[] objArr) { + if (objArr == null || objArr.length < 11) { + throw new IllegalArgumentException("Invalid result set: expected 11 columns"); + } this.id = objArr[0] != null ? BigInteger.valueOf(((Number) objArr[0]).longValue()) : null; // ... rest of assignments }src/main/java/com/wipro/fhir/repo/common/PatientEligibleForResourceCreationRepo.java (1)
118-133: Minor naming inconsistency in stored procedure methods.The method naming is inconsistent:
callMedicalHistorySpandcallOrganizationSpuse lowercaseSp, whilecallImmunizationSPandcallPractitionerSPuse uppercaseSP. Consider aligning with the existing convention in this file (uppercaseSP).🔎 Suggested naming alignment
//medicalHistory @Query(nativeQuery = true, value = "CALL db_iemr.FHIR_R_MedicalHistoryDetails(:visitCode_IN)") -List<Object[]> callMedicalHistorySp(@Param("visitCode_IN") BigInteger visitCode_IN); +List<Object[]> callMedicalHistorySP(@Param("visitCode_IN") BigInteger visitCode_IN); // ... @Query(nativeQuery = true, value = "CALL db_iemr.FHIR_R_OrganizationDetails(:visitCode_IN)") -List<Object[]> callOrganizationSp(@Param("visitCode_IN") BigInteger visitCode_IN); +List<Object[]> callOrganizationSP(@Param("visitCode_IN") BigInteger visitCode_IN);src/main/java/com/wipro/fhir/service/bundle_creation/ImmunizationRecordResourceBundle.java (1)
20-21: Inconsistent method naming convention.
PopulateImmunizationResourceBundleuses PascalCase whileprocessImmunizationRecordBundleuses camelCase. Java convention recommends camelCase for method names.Suggested fix
- String PopulateImmunizationResourceBundle(ResourceRequestHandler resourceRequestHandler, + String populateImmunizationResourceBundle(ResourceRequestHandler resourceRequestHandler, PatientEligibleForResourceCreation p) throws FHIRException;src/main/java/com/wipro/fhir/service/bundle_creation/OPConsultResourceBundle.java (1)
19-29: Inconsistent method naming convention.Methods
PopulateOpConsultCompositionandPopulateOPConsultRecordResourceBundleuse PascalCase, whileprocessOpConsultRecordBundleuses camelCase. For consistency and Java conventions, all methods should use camelCase.Suggested fix
- Composition PopulateOpConsultComposition(ResourceRequestHandler resourceRequestHandler, + Composition populateOpConsultComposition(ResourceRequestHandler resourceRequestHandler, PatientEligibleForResourceCreation p, Practitioner practitioner, Organization organization, List<Condition> conditionListChiefComplaints, List<Condition> conditionListDiagnosis, List<AllergyIntolerance> allergyList, FamilyMemberHistory familyMemberHistory, List<MedicationStatement> medicationStatement); int processOpConsultRecordBundle(ResourceRequestHandler resourceRequestHandler, PatientEligibleForResourceCreation p) throws FHIRException; - String PopulateOPConsultRecordResourceBundle(ResourceRequestHandler resourceRequestHandler, + String populateOPConsultRecordResourceBundle(ResourceRequestHandler resourceRequestHandler, PatientEligibleForResourceCreation p) throws FHIRException;src/main/java/com/wipro/fhir/service/bundle_creation/DischargeSummaryResourceBundle.java (1)
26-31: Method has many parameters (13 total).
PopulateDischargeSummaryCompositionhas 13 parameters which can be hard to maintain and read. Consider grouping related parameters into a DTO/parameter object for better maintainability.Also, method names
PopulateDischargeSummaryCompositionandPopulateDischargeSummaryResourceBundleshould use camelCase per Java conventions.src/main/java/com/wipro/fhir/service/resource_model/ImmunizationResource.java (1)
28-29: Consider making utility method static.
ImmunizationDataModelis injected as a Spring component but used solely to callgetImmunizationList(), which acts as a stateless utility. Consider making this method static inImmunizationDataModelto avoid unnecessary bean injection, consistent with typical data model patterns.src/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java (1)
40-57: Inconsistent null handling in constructor.Some fields have null checks (e.g., lines 42-43, 52-54) while others don't (lines 44-50, 55-56). This inconsistency could lead to issues if the stored procedure returns unexpected nulls. For example, if
objArr[2]is null, the cast(String) objArr[2]succeeds but ifobjArr[3]returns an incompatible type, it will throwClassCastException.Consider applying consistent null-safe handling as done in
ImmunizationDataModel:Suggested fix
public PractitionerDataModel(Object[] objArr) { this.benVisitID = objArr[0] != null ? ((Number) objArr[0]).intValue() : null; this.userID = objArr[1] != null ? ((Number) objArr[1]).intValue() : null; - this.fullName = (String) objArr[2]; - this.dob = (Date) objArr[3]; - this.employeeID = (String) objArr[4]; - this.contactNo = (String) objArr[5]; - this.emailID = (String) objArr[6]; - this.qualificationName = (String) objArr[7]; - this.designationName = (String) objArr[8]; - this.genderName = (String) objArr[9]; + this.fullName = objArr[2] != null ? (String) objArr[2] : null; + this.dob = objArr[3] != null ? (Date) objArr[3] : null; + this.employeeID = objArr[4] != null ? (String) objArr[4] : null; + this.contactNo = objArr[5] != null ? (String) objArr[5] : null; + this.emailID = objArr[6] != null ? (String) objArr[6] : null; + this.qualificationName = objArr[7] != null ? (String) objArr[7] : null; + this.designationName = objArr[8] != null ? (String) objArr[8] : null; + this.genderName = objArr[9] != null ? (String) objArr[9] : null; this.genderID = objArr[10] != null ? ((Number) objArr[10]).intValue() : null; this.serviceProviderID = objArr[11] != null ? ((Number) objArr[11]).intValue() : null; this.visitCode = objArr[12] != null ? ((Number) objArr[12]).longValue() : null; - this.createdBy = (String) objArr[13]; - this.createdDate = (Timestamp) objArr[14]; + this.createdBy = objArr[13] != null ? (String) objArr[13] : null; + this.createdDate = objArr[14] != null ? (Timestamp) objArr[14] : null; }src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.java (1)
10-12: Consider makinggetOrganizationstatic or removing@Componentannotation.The class is annotated with
@Component(singleton bean), butgetOrganization()creates new instances and doesn't use any instance state. This is inconsistent—either the factory method should bestatic, or the class should not be a Spring bean (similar to how POJOs are typically designed).This pattern is also present in sibling classes (
PractitionerDataModel,ImmunizationDataModel), so consider a consistent approach across all data models.src/main/java/com/wipro/fhir/service/bundle_creation/PrescriptionResourceBundle.java (1)
16-24: Method names should use camelCase per Java conventions.Interface method names like
PopulatePrescriptionResourceBundleandPopulatePrescriptionCompositionuse PascalCase, which violates standard Java naming conventions. Methods should start with a lowercase letter (e.g.,populatePrescriptionResourceBundle).This pattern is consistent across other new bundle interfaces in this PR, so consider updating all of them together.
src/main/java/com/wipro/fhir/service/resource_model/PractitionerResource.java (2)
24-25: Unused imports.
ArrayListandListare imported but not used in this class.-import java.util.ArrayList; -import java.util.List;
103-115: Consider case-insensitive gender matching for robustness.The gender mapping uses exact string matching (
"Male","Female"). If the database returns variations like"male"or"MALE", the practitioner will incorrectly getUNKNOWNgender.🔎 Proposed fix for case-insensitive matching
// Gender if (practitionerData.getGenderName() != null) { - switch (practitionerData.getGenderName()) { - case "Male": + switch (practitionerData.getGenderName().toLowerCase()) { + case "male": practitioner.setGender(AdministrativeGender.MALE); break; - case "Female": + case "female": practitioner.setGender(AdministrativeGender.FEMALE); break; default: practitioner.setGender(AdministrativeGender.UNKNOWN); break; } }src/main/java/com/wipro/fhir/service/bundle_creation/DiagnosticRecordResourceBundleImpl.java (1)
241-244: Prefer!isEmpty()oversize() > 0.Minor stylistic improvement for readability.
- if (objArrResultSet != null && objArrResultSet.size() > 0) { + if (objArrResultSet != null && !objArrResultSet.isEmpty()) {src/main/java/com/wipro/fhir/service/bundle_creation/WellnessRecordResourceBundleImpl.java (2)
149-151: Consider caching theFhirContextinstance.
FhirContext.forR4()is an expensive operation that should ideally be performed once and reused. Creating it on every bundle serialization impacts performance.🔎 Proposed fix: Use a cached FhirContext
+ private static final FhirContext FHIR_CONTEXT = FhirContext.forR4(); + @Override public String PopulateWellnessRecordResourceBundle(ResourceRequestHandler resourceRequestHandler, PatientEligibleForResourceCreation p) throws FHIRException { // ... existing code ... - FhirContext ctx = FhirContext.forR4(); - IParser parser = ctx.newJsonParser(); + IParser parser = FHIR_CONTEXT.newJsonParser(); serializeBundle = parser.encodeResourceToString(wellnessBundle);
192-209: UseisEmpty()instead ofsize() > 0.The check
observationVitalList.size() > 0should use!observationVitalList.isEmpty()for better readability and slight performance improvement on some collection types.🔎 Proposed fix
- if (observationVitalList.size() > 0) { + if (!observationVitalList.isEmpty()) { for (Observation obs : observationVitalList) {src/main/java/com/wipro/fhir/service/bundle_creation/OPConsultResourceBundleImpl.java (2)
227-229: Cache theFhirContextinstance for better performance.Same issue as other bundle implementations -
FhirContext.forR4()is expensive and should be created once as a static field.
265-311: Consider grouping related entries into single sections.The current implementation creates a separate section for each chief complaint, diagnosis, allergy, etc. Typically, FHIR Compositions group related entries into a single section with multiple entries. This would reduce bundle size and improve readability.
🔎 Example for Chief Complaints section
- for(Condition condition: conditionListChiefComplaints) { - SectionComponent section1 = new SectionComponent(); - section1.setTitle("Chief complaints"); - section1.setCode(new CodeableConcept(new Coding("http://snomed.info/sct", "422843007", "Chief complaint section"))) - .addEntry(new Reference().setReference(condition.getIdElement().getValue())); - - sectionList.add(section1); - } + if (!conditionListChiefComplaints.isEmpty()) { + SectionComponent section1 = new SectionComponent(); + section1.setTitle("Chief complaints"); + section1.setCode(new CodeableConcept(new Coding("http://snomed.info/sct", "422843007", "Chief complaint section"))); + for (Condition condition : conditionListChiefComplaints) { + section1.addEntry(new Reference().setReference(condition.getIdElement().getValue())); + } + sectionList.add(section1); + }src/main/java/com/wipro/fhir/service/bundle_creation/PrescriptionResourceBundleImpl.java (2)
155-157: Cache theFhirContextinstance.Same performance concern as other bundle implementations. Consider extracting to a shared utility or static field.
62-62: Logger is declared but underutilized.The logger is declared but the visible code only uses
throw new FHIRException. Consider logging errors before throwing exceptions for better observability, or remove if unnecessary.src/main/java/com/wipro/fhir/service/bundle_creation/DischargeSummaryResourceBundleImpl.java (3)
279-281: Cache theFhirContextinstance.Same performance concern as other bundle implementations.
123-289: MethodPopulateDischargeSummaryResourceBundleis very long and complex.This method spans ~165 lines and handles multiple concerns: fetching resources, building bundle entries, and serialization. Consider extracting resource fetching and bundle entry creation into separate private methods for improved readability and maintainability.
331-417: Creating separate sections for each individual item.Similar to
OPConsultResourceBundleImpl, this creates a separate section for each chief complaint, physical exam finding, allergy, and medication request. Consider grouping related entries into single sections with multiple entries for a more standard FHIR Composition structure.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
pom.xmlsrc/main/environment/common_ci.propertiessrc/main/environment/common_dev.propertiessrc/main/environment/common_docker.propertiessrc/main/environment/common_example.propertiessrc/main/environment/common_test.propertiessrc/main/java/com/wipro/fhir/controller/generateresource/ResourceRequestGateway.javasrc/main/java/com/wipro/fhir/controller/test/Test.javasrc/main/java/com/wipro/fhir/data/resource_model/ImmunizationDataModel.javasrc/main/java/com/wipro/fhir/data/resource_model/MedicalHistoryDataModel.javasrc/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.javasrc/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.javasrc/main/java/com/wipro/fhir/repo/common/PatientEligibleForResourceCreationRepo.javasrc/main/java/com/wipro/fhir/service/bundle_creation/BundleValidator.javasrc/main/java/com/wipro/fhir/service/bundle_creation/DiagnosticRecordResourceBundle.javasrc/main/java/com/wipro/fhir/service/bundle_creation/DiagnosticRecordResourceBundleImpl.javasrc/main/java/com/wipro/fhir/service/bundle_creation/DischargeSummaryResourceBundle.javasrc/main/java/com/wipro/fhir/service/bundle_creation/DischargeSummaryResourceBundleImpl.javasrc/main/java/com/wipro/fhir/service/bundle_creation/ImmunizationRecordResourceBundle.javasrc/main/java/com/wipro/fhir/service/bundle_creation/ImmunizationRecordResourceBundleImpl.javasrc/main/java/com/wipro/fhir/service/bundle_creation/OPConsultResourceBundle.javasrc/main/java/com/wipro/fhir/service/bundle_creation/OPConsultResourceBundleImpl.javasrc/main/java/com/wipro/fhir/service/bundle_creation/PrescriptionResourceBundle.javasrc/main/java/com/wipro/fhir/service/bundle_creation/PrescriptionResourceBundleImpl.javasrc/main/java/com/wipro/fhir/service/bundle_creation/WellnessRecordResourceBundle.javasrc/main/java/com/wipro/fhir/service/bundle_creation/WellnessRecordResourceBundleImpl.javasrc/main/java/com/wipro/fhir/service/common/CommonServiceImpl.javasrc/main/java/com/wipro/fhir/service/resource_gateway/DiagnosticReportRecord.javasrc/main/java/com/wipro/fhir/service/resource_gateway/DiagnosticReportRecordImpl.javasrc/main/java/com/wipro/fhir/service/resource_gateway/OPConsultRecordBundle.javasrc/main/java/com/wipro/fhir/service/resource_gateway/OPConsultRecordBundleImpl.javasrc/main/java/com/wipro/fhir/service/resource_gateway/PrescriptionRecordBundle.javasrc/main/java/com/wipro/fhir/service/resource_gateway/PrescriptionRecordBundleImpl.javasrc/main/java/com/wipro/fhir/service/resource_model/DiagnosticReportResource.javasrc/main/java/com/wipro/fhir/service/resource_model/EncounterResource.javasrc/main/java/com/wipro/fhir/service/resource_model/ImmunizationResource.javasrc/main/java/com/wipro/fhir/service/resource_model/MedicalHistoryResource.javasrc/main/java/com/wipro/fhir/service/resource_model/MedicationRequestResource.javasrc/main/java/com/wipro/fhir/service/resource_model/ObservationResource.javasrc/main/java/com/wipro/fhir/service/resource_model/OrganizationResource.javasrc/main/java/com/wipro/fhir/service/resource_model/PatientResource.javasrc/main/java/com/wipro/fhir/service/resource_model/PractitionerResource.java
💤 Files with no reviewable changes (8)
- src/main/java/com/wipro/fhir/service/resource_gateway/PrescriptionRecordBundle.java
- src/main/java/com/wipro/fhir/service/resource_gateway/DiagnosticReportRecord.java
- src/main/java/com/wipro/fhir/service/resource_gateway/PrescriptionRecordBundleImpl.java
- src/main/java/com/wipro/fhir/service/resource_model/DiagnosticReportResource.java
- src/main/java/com/wipro/fhir/service/resource_gateway/OPConsultRecordBundle.java
- src/main/java/com/wipro/fhir/service/resource_gateway/DiagnosticReportRecordImpl.java
- src/main/java/com/wipro/fhir/service/resource_model/PatientResource.java
- src/main/java/com/wipro/fhir/service/resource_gateway/OPConsultRecordBundleImpl.java
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-12-10T13:24:01.839Z
Learnt from: helenKaryamsetty
Repo: PSMRI/FHIR-API PR: 52
File: src/main/environment/common_example.properties:87-94
Timestamp: 2024-12-10T13:24:01.839Z
Learning: In the FHIR-API project, URLs are maintained directly in the properties files rather than using variables, so it's acceptable to use direct URLs instead of variables.
Applied to files:
src/main/environment/common_test.propertiessrc/main/environment/common_example.propertiessrc/main/environment/common_dev.properties
📚 Learning: 2025-06-17T12:45:58.739Z
Learnt from: vishwab1
Repo: PSMRI/FHIR-API PR: 107
File: src/main/java/com/wipro/fhir/config/CorsConfig.java:24-26
Timestamp: 2025-06-17T12:45:58.739Z
Learning: FHIR APIs often require allowing all headers (`allowedHeaders("*")`) in CORS configuration due to the need for custom headers for various integrations and healthcare standards compliance, making restrictive header lists impractical.
Applied to files:
src/main/environment/common_example.properties
📚 Learning: 2024-11-20T07:41:40.038Z
Learnt from: helenKaryamsetty
Repo: PSMRI/FHIR-API PR: 46
File: src/main/java/com/wipro/fhir/data/mongo/care_context/SaveFacilityIdForVisit.java:7-8
Timestamp: 2024-11-20T07:41:40.038Z
Learning: In the `SaveFacilityIdForVisit` class (`src/main/java/com/wipro/fhir/data/mongo/care_context/SaveFacilityIdForVisit.java`), the handling of the MongoDB ID field and indexing is provided in another module and is not required in this class.
Applied to files:
src/main/java/com/wipro/fhir/service/bundle_creation/WellnessRecordResourceBundleImpl.javasrc/main/java/com/wipro/fhir/service/bundle_creation/OPConsultResourceBundleImpl.javasrc/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java
📚 Learning: 2024-11-20T07:45:51.578Z
Learnt from: helenKaryamsetty
Repo: PSMRI/FHIR-API PR: 46
File: src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java:365-381
Timestamp: 2024-11-20T07:45:51.578Z
Learning: In `CommonServiceImpl.java`, within the `addCareContextToMongo` method, prefer using direct switch-case statements for gender mapping instead of introducing separate enums.
Applied to files:
src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java
🧬 Code graph analysis (5)
src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.java (3)
src/main/java/com/wipro/fhir/data/resource_model/ImmunizationDataModel.java (1)
Data(12-59)src/main/java/com/wipro/fhir/data/resource_model/MedicalHistoryDataModel.java (1)
Data(14-61)src/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java (1)
Data(10-67)
src/main/java/com/wipro/fhir/data/resource_model/ImmunizationDataModel.java (3)
src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.java (1)
Component(10-77)src/main/java/com/wipro/fhir/data/resource_model/MedicalHistoryDataModel.java (1)
Data(14-61)src/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java (1)
Data(10-67)
src/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java (3)
src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.java (1)
Component(10-77)src/main/java/com/wipro/fhir/data/resource_model/ImmunizationDataModel.java (1)
Data(12-59)src/main/java/com/wipro/fhir/data/resource_model/MedicalHistoryDataModel.java (1)
Data(14-61)
src/main/java/com/wipro/fhir/service/bundle_creation/PrescriptionResourceBundleImpl.java (3)
src/main/java/com/wipro/fhir/service/resource_model/MedicationRequestResource.java (1)
Service(52-168)src/main/java/com/wipro/fhir/service/resource_model/PractitionerResource.java (1)
Service(43-142)src/main/java/com/wipro/fhir/service/resource_model/PatientResource.java (1)
Service(50-168)
src/main/java/com/wipro/fhir/data/resource_model/MedicalHistoryDataModel.java (3)
src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.java (1)
Component(10-77)src/main/java/com/wipro/fhir/data/resource_model/ImmunizationDataModel.java (1)
Data(12-59)src/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java (1)
Data(10-67)
🔇 Additional comments (21)
src/main/java/com/wipro/fhir/service/bundle_creation/BundleValidator.java (2)
1-23: LGTM!The static
FhirContextinitialization is appropriate sinceFhirContextis thread-safe and expensive to create.
31-42: LGTM!The validation support chain is properly constructed with the necessary components for FHIR R4 validation.
src/main/java/com/wipro/fhir/service/resource_model/MedicationRequestResource.java (1)
152-157: Good defensive coding practice.The null and empty check for
conditionDiagnosisprevents potentialNullPointerExceptionwhen the condition list is not provided. Empty lists are still set forreasonCodeandreasonReference, maintaining consistent behavior.src/main/environment/common_dev.properties (1)
108-109: LGTM!The new
systemUrlproperty follows the established placeholder pattern used for other configurable values in this file and aligns with the broader PR changes for HIP system URL configuration.src/main/environment/common_example.properties (1)
120-122: LGTM!The
systemUrlproperty addition is consistent with the pattern used across other environment configuration files in this PR.src/main/environment/common_ci.properties (1)
127-128: LGTM!The
systemUrlproperty correctly uses the@env.HIP_SYSTEM_URL@environment variable substitution pattern, consistent with other CI-specific configurations in this file.src/main/java/com/wipro/fhir/data/resource_model/MedicalHistoryDataModel.java (1)
49-59: LGTM!The
getMedicalListmethod follows the established pattern used by similar data model classes (e.g.,ImmunizationDataModel.getImmunizationList). The implementation correctly handles null/empty input.pom.xml (1)
186-200: LGTM - Jackson version updates.The Jackson dependencies are updated consistently from 2.14.2 to 2.15.2 across core, databind, and annotations artifacts.
src/main/java/com/wipro/fhir/repo/common/PatientEligibleForResourceCreationRepo.java (2)
120-121: Verify stored procedure query format forcallMedicalHistorySp.Unlike other SP calls in this file (e.g.,
callAllergySP,callConditionSP), this query doesn't include output parameter placeholders (@0, @1, ...). Verify this is intentional based on howFHIR_R_MedicalHistoryDetailsreturns data.
128-129: Same verification needed forcallOrganizationSp.This query also lacks output parameter placeholders. Confirm the SP
FHIR_R_OrganizationDetailsreturns a result set directly without OUT parameters.src/main/java/com/wipro/fhir/service/resource_model/ObservationResource.java (1)
66-74: All call sites have been updated correctly for the signature change.The method signature change from
getObservationVitals(Patient, Encounter, ResourceRequestHandler)togetObservationVitals(Patient, ResourceRequestHandler)is implemented correctly. The sole call site inWellnessRecordResourceBundleImpl.java(lines 109-110) passes only thePatientandResourceRequestHandlerparameters, matching the new signature.src/main/java/com/wipro/fhir/service/resource_model/ImmunizationResource.java (1)
56-57: Undefined reference to commonService.getUUID().Line 57 calls
commonService.getUUID(), but thecommonServicefield (lines 25-26) appears to be injected. However, this seems inconsistent with the earlier observation. Verify thatcommonServiceis properly used here.src/main/java/com/wipro/fhir/service/bundle_creation/DiagnosticRecordResourceBundle.java (1)
14-27: Interface design is consistent with the new bundle architecture.The interface follows the same pattern as
PrescriptionResourceBundleand other bundle interfaces. The same PascalCase naming convention observation applies here (methods should use camelCase).src/main/java/com/wipro/fhir/data/resource_model/ImmunizationDataModel.java (1)
33-47: Constructor uses safe numeric conversions correctly.The constructor correctly uses the
((Number) objArr[i]).intValue()pattern for safe numeric type conversion, which is consistent withPractitionerDataModelandMedicalHistoryDataModel.Consider adding a bounds check (
objArr.length >= 13) at the start of the constructor for defensive programming, similar to the suggestion forOrganizationDataModel.src/main/java/com/wipro/fhir/service/resource_model/PractitionerResource.java (1)
55-66: Data-driven practitioner resource creation looks good.The refactoring from hard-coded dummy data to data-driven resource creation via the stored procedure and
PractitionerDataModelis a solid improvement. The null/empty validation with a descriptive exception message is appropriate.src/main/java/com/wipro/fhir/controller/generateresource/ResourceRequestGateway.java (2)
76-88: UnusedAuthorizationheader parameter.The
Authorizationheader is captured as a parameter but never used in this method (and similarly ingetDiagnosticReportRecordandgetPrescriptionRecord). If authorization is handled elsewhere (e.g., by a filter), consider removing the unused parameter to avoid confusion. If authorization should be validated here, it's currently missing.
57-62: Migration to new bundle interfaces looks correct.The field types and injected dependencies have been correctly updated to use the new
bundle_creationinterfaces (OPConsultResourceBundle,PrescriptionResourceBundle,DiagnosticRecordResourceBundle). The method call updates align with the new API signatures.src/main/java/com/wipro/fhir/service/bundle_creation/DiagnosticRecordResourceBundleImpl.java (1)
196-216: Potential NPE:PatientEligibleForResourceCreation pmay be null.In
PopulateDiagnosticReportComposition,p.getBeneficiaryId()is called on line 212, butpcan benullbased on the interface contract (seeResourceRequestGatewaypassingnull). Add a null check or handle this case gracefully.🔎 Proposed fix
public Composition PopulateDiagnosticReportComposition(ResourceRequestHandler resourceRequestHandler, PatientEligibleForResourceCreation p, List<DiagnosticReport> diagnosticReports, Practitioner practitioner, Organization organization) { + if (p == null) { + throw new IllegalArgumentException("PatientEligibleForResourceCreation cannot be null"); + } Composition composition = new Composition();Alternatively, if
pbeing null is a valid case, derive the beneficiary ID from another source (e.g.,resourceRequestHandler).⛔ Skipped due to learnings
Learnt from: helenKaryamsetty Repo: PSMRI/FHIR-API PR: 46 File: src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java:328-329 Timestamp: 2024-11-20T07:47:14.579Z Learning: In the `processResourceOperation` method of `CommonServiceImpl.java`, the `pVisit` variable is guaranteed to be non-null, so null checks on `pVisit` are unnecessary.src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java (1)
233-239: OP Consult processing lacks error handling for the condition check.If
p.getVisitCategory()returns null, theequalsIgnoreCasecalls will throw aNullPointerException. Consider adding a null check.🔎 Proposed fix
// 1. OP consult resource bundle - if (p.getVisitCategory().equalsIgnoreCase("General OPD") - || p.getVisitCategory().equalsIgnoreCase("General OPD (QC)")) { + String visitCategory = p.getVisitCategory(); + if (visitCategory != null && + (visitCategory.equalsIgnoreCase("General OPD") || visitCategory.equalsIgnoreCase("General OPD (QC)"))) {⛔ Skipped due to learnings
Learnt from: helenKaryamsetty Repo: PSMRI/FHIR-API PR: 46 File: src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java:328-329 Timestamp: 2024-11-20T07:47:14.579Z Learning: In the `processResourceOperation` method of `CommonServiceImpl.java`, the `pVisit` variable is guaranteed to be non-null, so null checks on `pVisit` are unnecessary.Learnt from: helenKaryamsetty Repo: PSMRI/FHIR-API PR: 57 File: src/main/java/com/wipro/fhir/service/facility/FacilityServiceImpl.java:89-92 Timestamp: 2025-01-07T11:20:36.850Z Learning: In the FHIR-API project, visitCode validation (null checks) is handled at the UI layer, and therefore doesn't need to be duplicated in the service layer methods like `saveAbdmFacilityId`.src/main/java/com/wipro/fhir/service/bundle_creation/ImmunizationRecordResourceBundleImpl.java (1)
100-100: The parameter order in the Coding constructor is correct. HAPI FHIR's Coding constructor expects parameters in the order: theSystem, theCode, theDisplay, and the code at line 100 follows this standard order with system, code ("R"), and display ("restricted"). No correction is needed.src/main/java/com/wipro/fhir/service/resource_model/OrganizationResource.java (1)
76-81: Make the ABDM facility identifier system URL configurable.The production Health Facility Registry uses https://facility.ndhm.gov.in, but the code hardcodes the sandbox URL https://facilitysbx.ndhm.gov.in. Identifier system URLs should be externalized as configurable properties to support different environments. Extract this to a Spring @Value-injected property to support both sandbox and production deployments.
⛔ Skipped due to learnings
Learnt from: helenKaryamsetty Repo: PSMRI/FHIR-API PR: 52 File: src/main/environment/common_example.properties:87-94 Timestamp: 2024-12-10T13:24:01.839Z Learning: In the FHIR-API project, URLs are maintained directly in the properties files rather than using variables, so it's acceptable to use direct URLs instead of variables.
| <dependency> | ||
| <groupId>ca.uhn.hapi.fhir</groupId> | ||
| <artifactId>hapi-fhir-structures-r4</artifactId> | ||
| <version>6.10.0</version> | ||
| <version>8.4.0</version> | ||
| </dependency> |
There was a problem hiding this comment.
Verify HAPI FHIR 8.4.0 compatibility with the codebase.
This is a major version upgrade from 6.10.0 to 8.4.0 with documented breaking changes. Notable API changes include package migration from javax.* to jakarta.*, removal of compatibility shims, changes to contained resource ID handling, and modifications to Device resource search parameter compartment rules. Ensure the codebase compiles and runs correctly with this update, particularly if using interceptors or custom search logic.
🤖 Prompt for AI Agents
In pom.xml around lines 204–208 you upgraded hapi-fhir-structures-r4 from 6.10.0
to 8.4.0 which contains breaking changes; verify and fix code to compile and run
against 8.4.0 by updating all Java EE imports from javax.* to jakarta.* (or add
appropriate migration/shaded artifacts), review and adapt any interceptor
implementations to the new interceptor APIs, adjust code that handles contained
resource IDs and Device search/compartment logic to match the new behavior, run
a full mvn clean compile/test/integration-test cycle to find errors, and either
fix call sites or pin to a compatible HAPI FHIR version if changes are too
invasive.
There was a problem hiding this comment.
@coderabbitai these versions are completely aligned with codebase
There was a problem hiding this comment.
@helenKaryamsetty, thank you for confirming! I appreciate you verifying that the HAPI FHIR 8.4.0 upgrade is compatible with the codebase.
src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.java
Show resolved
Hide resolved
src/main/java/com/wipro/fhir/service/bundle_creation/BundleValidator.java
Show resolved
Hide resolved
src/main/java/com/wipro/fhir/service/bundle_creation/DiagnosticRecordResourceBundleImpl.java
Show resolved
Hide resolved
| // 6. Discharge Summary | ||
| int dischargeSummary = dischargeSummaryResourceBundle | ||
| .processDischargeSummaryRecordBundle(resourceRequestHandler, p); | ||
| if (dischargeSummary <= 0) | ||
| processed = false; | ||
| logger.info(" The value of dischargeSummary proceesed: " + processed); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "CommonServiceImpl.java" -type fRepository: PSMRI/FHIR-API
Length of output: 126
🏁 Script executed:
fd CommonServiceImpl.javaRepository: PSMRI/FHIR-API
Length of output: 124
🏁 Script executed:
cat -n src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java | head -300 | tail -100Repository: PSMRI/FHIR-API
Length of output: 5124
🏁 Script executed:
rg "hasDischargeSummary|hasDischarge" --type javaRepository: PSMRI/FHIR-API
Length of output: 40
🏁 Script executed:
rg "careContextRepo\." src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java | head -10Repository: PSMRI/FHIR-API
Length of output: 406
Discharge summary is processed unconditionally, unlike other bundles with eligibility checks.
Lines 242-277 show that diagnostic, prescription, wellness, and immunization bundles are only processed if corresponding eligibility checks pass (hasLabTests, hasPrescription, hasPhyVitals, hasVaccineDetails). Lines 280-284 call discharge summary processing with no conditional check. If discharge summary processing fails for visits without discharge data, the entire processed flag is set to false (line 283), preventing the visit from being marked as complete.
Either add a conditional eligibility check for discharge summary (if a corresponding method exists in careContextRepo), or ensure the discharge summary implementation gracefully handles visits without discharge data.
🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java around
lines 279 to 284, discharge summary processing is invoked unconditionally which
can flip processed to false for visits without discharge data; either wrap the
dischargeSummaryResourceBundle.processDischargeSummaryRecordBundle(...) call in
a conditional that checks eligibility (e.g., use an appropriate
careContextRepo.hasDischargeSummary(...) or similarly named method) before
calling it, or modify the processDischargeSummaryRecordBundle method to return a
neutral/success value when no discharge data exists so it does not set processed
= false; implement one of these fixes and update the logger call accordingly.
| @Autowired | ||
| private CommonService commonService; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Unused dependency injection.
CommonService is autowired but never used in this class. Either remove the injection or the field is missing its intended usage.
Suggested fix
- @Autowired
- private CommonService commonService;
-Note: If commonService.getUUID() was intended to be used at line 57, then the current code has a bug since commonService is defined but not invoked there.
📝 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.
| @Autowired | |
| private CommonService commonService; |
🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/service/resource_model/ImmunizationResource.java
around lines 25-26, the CommonService field is @Autowired but never used; either
remove the unused injection to clean up the class or, if the intent was to call
CommonService (e.g., commonService.getUUID()) at the location referenced in the
review (around line 57), add the missing invocation there and replace any
placeholder UUID generation with commonService.getUUID(); ensure imports and
null-safety are handled accordingly.
| CodeableConcept medCC = new CodeableConcept(); | ||
| medCC.addCoding(new Coding() | ||
| .setSystem("http://snomed.info/sct") | ||
| .setCode(" ") | ||
| .setDisplay(med.getCurrentMedication())); // scts code so kept only the name | ||
|
|
||
| medCC.setText(med.getCurrentMedication()); | ||
| ms.setMedication(medCC); |
There was a problem hiding this comment.
Invalid SNOMED code usage.
Setting a whitespace string " " as the SNOMED code (line 60) is not a valid approach. If no SNOMED code is available, consider either:
- Omitting the Coding entirely and only setting the text
- Using a "unknown" or appropriate null-flavor pattern
The current approach may cause FHIR validation failures.
Suggested fix - omit Coding when no code available
CodeableConcept medCC = new CodeableConcept();
- medCC.addCoding(new Coding()
- .setSystem("http://snomed.info/sct")
- .setCode(" ")
- .setDisplay(med.getCurrentMedication())); // scts code so kept only the name
-
+ // Only set text since SNOMED code is not available
medCC.setText(med.getCurrentMedication());
ms.setMedication(medCC);📝 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.
| CodeableConcept medCC = new CodeableConcept(); | |
| medCC.addCoding(new Coding() | |
| .setSystem("http://snomed.info/sct") | |
| .setCode(" ") | |
| .setDisplay(med.getCurrentMedication())); // scts code so kept only the name | |
| medCC.setText(med.getCurrentMedication()); | |
| ms.setMedication(medCC); | |
| CodeableConcept medCC = new CodeableConcept(); | |
| // Only set text since SNOMED code is not available | |
| medCC.setText(med.getCurrentMedication()); | |
| ms.setMedication(medCC); |
🤖 Prompt for AI Agents
In
src/main/java/com/wipro/fhir/service/resource_model/MedicalHistoryResource.java
around lines 57 to 64, the code currently sets a whitespace string " " as the
SNOMED code which is invalid and can break FHIR validation; change the logic so
that if a valid SNOMED code is available you create and add a Coding with system
("http://snomed.info/sct"), code and display, otherwise do not add a Coding at
all and only set the CodeableConcept text (or, if your domain requires, apply a
defined "unknown"/null-flavor pattern consistently); ensure you guard the Coding
creation with a non-empty check on the code before calling addCoding.
| int index = 0; | ||
| for (MedicationRequestDataModel obj : medicationRequestList) { | ||
| index++; | ||
| medicationRequest = new MedicationRequest(); | ||
|
|
||
| codeableConcept = new CodeableConcept(); | ||
| c = new Coding(); | ||
| cList = new ArrayList<>(); | ||
|
|
||
| medicationRequest.setId("MedicationRequest/" + commonService.getUUID()); | ||
| medicationRequest.setId("MedicationRequest - " + index + "/" + commonService.getUUID()); |
There was a problem hiding this comment.
Non-standard FHIR resource ID format violates specification constraints.
FHIR logical IDs must contain only ASCII letters, numerals, hyphens (-), and dots (.), with a 64-character length limit. The ID format "MedicationRequest - " + index + "/" + commonService.getUUID() violates these constraints:
- Spaces are not permitted: The space character in "MedicationRequest - " is outside the allowed character set.
- Likely exceeds 64-character limit: Including the resource type label, index, slash separator, and full UUID results in an ID that exceeds the maximum length.
- Interoperability risk: FHIR validators and downstream systems may reject or misparse these IDs.
Use a format like MedicationRequest-{index}-{uuid} or assign UUIDs directly without prefixes instead.
🤖 Prompt for AI Agents
In
src/main/java/com/wipro/fhir/service/resource_model/MedicationRequestResource.java
around lines 93-102, the resource ID is built as "MedicationRequest - " + index
+ "/" + commonService.getUUID(), which contains spaces and a slash and may
exceed FHIR's 64-char limit; replace this with a FHIR-safe ID such as
"MedicationRequest-{index}-{uuid}" (no spaces, use hyphens or dots only), or
simply use the UUID alone, and enforce/validate the result is <=64 characters
(truncate or fall back to UUID-only if necessary) so the ID contains only ASCII
letters, digits, hyphens and dots and meets length constraints.
| public Organization getOrganizationResource(ResourceRequestHandler resourceRequestHandler) throws FHIRException { | ||
|
|
||
| public Organization getOrganization() { | ||
| return generateOrganizationResource(); | ||
| List<Object[]> rsObj = patientEligibleForResourceCreationRepo.callOrganizationSp(resourceRequestHandler.getVisitCode()); | ||
|
|
||
| OrganizationDataModel orgData = organizationDataModel.getOrganization(rsObj.get(0)); | ||
|
|
||
| if (orgData != null) { | ||
| return generateOrganizationResource(orgData); | ||
| } else { | ||
| throw new FHIRException("Organization not found"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential IndexOutOfBoundsException if SP returns empty results.
Line 50 calls rsObj.get(0) without checking if the list is empty. If callOrganizationSp returns an empty list, this will throw an IndexOutOfBoundsException.
🔎 Proposed fix
public Organization getOrganizationResource(ResourceRequestHandler resourceRequestHandler) throws FHIRException {
List<Object[]> rsObj = patientEligibleForResourceCreationRepo.callOrganizationSp(resourceRequestHandler.getVisitCode());
+ if (rsObj == null || rsObj.isEmpty()) {
+ throw new FHIRException("Organization not found for visit code: " + resourceRequestHandler.getVisitCode());
+ }
+
OrganizationDataModel orgData = organizationDataModel.getOrganization(rsObj.get(0));
if (orgData != null) {
return generateOrganizationResource(orgData);
} else {
throw new FHIRException("Organization not found");
}
}📝 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 Organization getOrganizationResource(ResourceRequestHandler resourceRequestHandler) throws FHIRException { | |
| public Organization getOrganization() { | |
| return generateOrganizationResource(); | |
| List<Object[]> rsObj = patientEligibleForResourceCreationRepo.callOrganizationSp(resourceRequestHandler.getVisitCode()); | |
| OrganizationDataModel orgData = organizationDataModel.getOrganization(rsObj.get(0)); | |
| if (orgData != null) { | |
| return generateOrganizationResource(orgData); | |
| } else { | |
| throw new FHIRException("Organization not found"); | |
| } | |
| } | |
| public Organization getOrganizationResource(ResourceRequestHandler resourceRequestHandler) throws FHIRException { | |
| List<Object[]> rsObj = patientEligibleForResourceCreationRepo.callOrganizationSp(resourceRequestHandler.getVisitCode()); | |
| if (rsObj == null || rsObj.isEmpty()) { | |
| throw new FHIRException("Organization not found for visit code: " + resourceRequestHandler.getVisitCode()); | |
| } | |
| OrganizationDataModel orgData = organizationDataModel.getOrganization(rsObj.get(0)); | |
| if (orgData != null) { | |
| return generateOrganizationResource(orgData); | |
| } else { | |
| throw new FHIRException("Organization not found"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/service/resource_model/OrganizationResource.java
around lines 46-57, the code calls rsObj.get(0) without verifying rsObj is
non-null and non-empty which can cause an IndexOutOfBoundsException; update the
method to first validate rsObj (and its size) before accessing element 0, handle
a null/empty result by throwing a FHIRException with a clear message (e.g., "No
organization data returned for visitCode: ...") or return an appropriate
response, then only call organizationDataModel.getOrganization(...) and
generateOrganizationResource(...) when data exists.
| spring.redis.host=${REDIS_HOST} | ||
| spring.redis.host=${REDIS_HOST} | ||
|
|
||
| systemUrl= <Enter HIP request URL> |
| logging.level.com.iemr=DEBUG | ||
| logging.level.org.springframework=INFO | ||
|
|
||
| systemUrl= <Enter HIP request URL> |
There was a problem hiding this comment.
This variable is so ambiguous. 1 year later, nobody will know.
What "system"?
|
3 participants
📋 Description
JIRA ID: AMM-1856, AMM-1885, AMM-1886, AMM-1884, AMM-1803
✅ 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
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.