-
Notifications
You must be signed in to change notification settings - Fork 235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jian | BAH-460 | add api for search similar patient using PersonService #29
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,8 +22,13 @@ | |
import org.openmrs.Patient; | ||
import org.openmrs.PatientIdentifier; | ||
import org.openmrs.PatientIdentifierType; | ||
import org.openmrs.Person; | ||
import org.openmrs.PersonName; | ||
import org.openmrs.RelationshipType; | ||
import org.openmrs.api.context.Context; | ||
import org.openmrs.api.db.hibernate.HibernatePatientDAO; | ||
import org.openmrs.api.db.hibernate.PersonLuceneQuery; | ||
import org.openmrs.api.db.hibernate.search.LuceneQuery; | ||
import org.openmrs.module.bahmniemrapi.visitlocation.BahmniVisitLocationServiceImpl; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.stereotype.Repository; | ||
|
@@ -100,6 +105,66 @@ public List<PatientResponse> getPatientsUsingLuceneSearch(String identifier, Str | |
return patientResponses; | ||
} | ||
|
||
@Override | ||
public List<PatientResponse> getSimilarPatientsUsingLuceneSearch(String identifier, String name, String gender, String customAttribute, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should really take some kind of search object (or a PatientProfile or BahmniPatient as I've suggested elsewhere) as its single parameter. As written here this seems like this method sign will need to be frequently modified in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I'm surprised to see so much logic here, e.g. for program attributes. Is that required to address similar patient search when creating a new patient? |
||
String addressFieldName, String addressFieldValue, Integer length, | ||
Integer offset, String[] customAttributeFields, String programAttributeFieldValue, | ||
String programAttributeFieldName, String[] addressSearchResultFields, | ||
String[] patientSearchResultFields, String loginLocationUuid, | ||
Boolean filterPatientsByLocation, Boolean filterOnAllIdentifiers) { | ||
|
||
validateSearchParams(customAttributeFields, programAttributeFieldName, addressFieldName); | ||
PatientResponseMapper patientResponseMapper = new PatientResponseMapper(Context.getVisitService(),new BahmniVisitLocationServiceImpl(Context.getLocationService())); | ||
|
||
List<Patient> patients = getPatientsByNameAndGender(name, gender, length); | ||
List<Integer> patientIds = patients.stream().map(patient -> patient.getPatientId()).collect(toList()); | ||
Map<Object, Object> programAttributes = Context.getService(BahmniProgramWorkflowService.class).getPatientProgramAttributeByAttributeName(patientIds, programAttributeFieldName); | ||
Set<Integer> uniquePatientIds = new HashSet<>(); | ||
List<PatientResponse> patientResponses = patients.stream() | ||
.map(patient -> { | ||
if(!uniquePatientIds.contains(patient.getPatientId())) { | ||
PatientResponse patientResponse = patientResponseMapper.map(patient, loginLocationUuid, patientSearchResultFields, addressSearchResultFields, | ||
programAttributes.get(patient.getPatientId())); | ||
uniquePatientIds.add(patient.getPatientId()); | ||
return patientResponse; | ||
}else | ||
return null; | ||
}).filter(Objects::nonNull) | ||
.collect(toList()); | ||
return patientResponses; | ||
} | ||
|
||
|
||
private List<Patient> getPatientsByNameAndGender(String name, String gender, Integer length) { | ||
HibernatePatientDAO patientDAO = new HibernatePatientDAO(); | ||
patientDAO.setSessionFactory(sessionFactory); | ||
List<Patient> patients = new ArrayList<Patient>(); | ||
String query = LuceneQuery.escapeQuery(name); | ||
PersonLuceneQuery personLuceneQuery = new PersonLuceneQuery(sessionFactory); | ||
LuceneQuery<PersonName> nameQuery = personLuceneQuery.getPatientNameQueryWithOrParser(query, false); | ||
/* person.gender does not work somehow in LuceneQuery, so the dirty way is to filter result with person's gender */ | ||
// if(gender != null && !gender.isEmpty()){ | ||
// nameQuery.include("person.gender", gender); | ||
// } | ||
List<PersonName> persons = nameQuery.list().stream() | ||
.filter( | ||
personName -> | ||
personName.getPreferred() | ||
&& checkGender(personName.getPerson(), gender) | ||
).collect(toList()); | ||
persons = persons.subList(0, Math.min(length, persons.size())); | ||
persons.forEach(person -> patients.add(patientDAO.getPatient(person.getPerson().getPersonId()))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @angshu @djazayeri we do not want to hit the database just to get the Patients from the Persons. We used the lucence index to get the PersonName's of similar Patients via cc @eyalgo (who is picking up on the work done by Jian with Martina) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. answering this late, I haven't ever used OpenMRS's lucene searching, so I don't know offhand how this works. |
||
return patients; | ||
} | ||
|
||
private Boolean checkGender(Person person, String gender) { | ||
if(gender != null && !gender.isEmpty()){ | ||
return gender.equals(person.getGender()); | ||
} else { | ||
return true; | ||
} | ||
} | ||
|
||
private List<PatientIdentifier> getPatientIdentifiers(String identifier, Boolean filterOnAllIdentifiers, Integer offset, Integer length) { | ||
FullTextSession fullTextSession = Search.getFullTextSession(sessionFactory.getCurrentSession()); | ||
QueryBuilder queryBuilder = fullTextSession.getSearchFactory().buildQueryBuilder().forEntity(PatientIdentifier.class).get(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,27 @@ | ||
package org.bahmni.module.bahmnicore.service.impl; | ||
|
||
import org.bahmni.module.bahmnicore.contract.patient.PatientSearchParameters; | ||
import org.bahmni.module.bahmnicore.contract.patient.mapper.PatientResponseMapper; | ||
import org.bahmni.module.bahmnicore.contract.patient.response.PatientConfigResponse; | ||
import org.bahmni.module.bahmnicore.contract.patient.response.PatientResponse; | ||
import org.bahmni.module.bahmnicore.dao.PatientDao; | ||
import org.bahmni.module.bahmnicore.service.BahmniPatientService; | ||
import org.openmrs.Concept; | ||
import org.openmrs.Patient; | ||
import org.openmrs.Person; | ||
import org.openmrs.PersonAttributeType; | ||
import org.openmrs.RelationshipType; | ||
import org.openmrs.api.ConceptService; | ||
import org.openmrs.api.PersonService; | ||
import org.openmrs.api.context.Context; | ||
import org.openmrs.module.bahmniemrapi.visitlocation.BahmniVisitLocationServiceImpl; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.context.annotation.Lazy; | ||
import org.springframework.stereotype.Service; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Set; | ||
|
||
@Service | ||
@Lazy //to get rid of cyclic dependencies | ||
|
@@ -83,6 +89,25 @@ public List<PatientResponse> luceneSearch(PatientSearchParameters searchParamete | |
searchParameters.getFilterPatientsByLocation(), searchParameters.getFilterOnAllIdentifiers()); | ||
} | ||
|
||
@Override | ||
public List<PatientResponse> searchSimilarPatients(PatientSearchParameters searchParameters) { | ||
return patientDao.getSimilarPatientsUsingLuceneSearch(searchParameters.getIdentifier(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not just pass PatientSearchParameters to the DAO? |
||
searchParameters.getName(), | ||
searchParameters.getGender(), | ||
searchParameters.getCustomAttribute(), | ||
searchParameters.getAddressFieldName(), | ||
searchParameters.getAddressFieldValue(), | ||
searchParameters.getLength(), | ||
searchParameters.getStart(), | ||
searchParameters.getPatientAttributes(), | ||
searchParameters.getProgramAttributeFieldValue(), | ||
searchParameters.getProgramAttributeFieldName(), | ||
searchParameters.getAddressSearchResultFields(), | ||
searchParameters.getPatientSearchResultFields(), | ||
searchParameters.getLoginLocationUuid(), | ||
searchParameters.getFilterPatientsByLocation(), searchParameters.getFilterOnAllIdentifiers()); | ||
} | ||
|
||
@Override | ||
public List<Patient> get(String partialIdentifier, boolean shouldMatchExactPatientId) { | ||
return patientDao.getPatients(partialIdentifier, shouldMatchExactPatientId); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -213,4 +213,35 @@ public void shouldNotReturnDuplicatePatientsEvenIfTwoIdentifiersMatches() { | |
assertTrue(patient.getExtraIdentifiers().contains("200006")); | ||
} | ||
|
||
@Test | ||
public void shouldSearchSimilarPatientByPatientName() { | ||
String[] addressResultFields = {"city_village"}; | ||
List<PatientResponse> patients = patientDao.getSimilarPatientsUsingLuceneSearch("", "Peet", "", null, "city_village", "", 100, 0, null,"",null,addressResultFields,null, "c36006e5-9fbb-4f20-866b-0ece245615a1", false, false); | ||
PatientResponse patient1 = patients.get(0); | ||
PatientResponse patient2 = patients.get(1); | ||
|
||
assertEquals(2, patients.size()); | ||
assertEquals(patient1.getGivenName(), "Horatio"); | ||
assertEquals(patient1.getMiddleName(), "Peeter"); | ||
assertEquals(patient1.getFamilyName(), "Sinha"); | ||
assertEquals(patient2.getGivenName(), "John"); | ||
assertEquals(patient2.getMiddleName(), "Peeter"); | ||
assertEquals(patient2.getFamilyName(), "Sinha"); | ||
} | ||
|
||
@Test | ||
public void shouldSearchSimilarPatientByPatientNameAndGender() { | ||
String[] addressResultFields = {"city_village"}; | ||
List<PatientResponse> patients = patientDao.getSimilarPatientsUsingLuceneSearch("", "Peet", "F", null, "city_village", "", 100, 0, null,"",null,addressResultFields,null, "c36006e5-9fbb-4f20-866b-0ece245615a1", false, false); | ||
PatientResponse patient1 = patients.get(0); | ||
|
||
for(PatientResponse response: patients) { | ||
System.out.println(response.getGivenName() + " " + response.getMiddleName() + " " + response.getFamilyName()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this before merging. |
||
} | ||
assertEquals(1, patients.size()); | ||
assertEquals(patient1.getGivenName(), "John"); | ||
assertEquals(patient1.getMiddleName(), "Peeter"); | ||
assertEquals(patient1.getFamilyName(), "Sinha"); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
package org.bahmni.module.bahmnicore.service.impl; | ||
|
||
import org.bahmni.module.bahmnicore.contract.patient.PatientSearchParameters; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. presumably this change isn't necessary since there's no changes to the class itself |
||
import org.bahmni.module.bahmnicore.contract.patient.mapper.PatientResponseMapper; | ||
import org.bahmni.module.bahmnicore.contract.patient.response.PatientConfigResponse; | ||
import org.bahmni.module.bahmnicore.dao.PatientDao; | ||
import org.junit.Before; | ||
|
@@ -67,4 +69,5 @@ public void shouldGetPatientByPartialIdentifier() throws Exception { | |
bahmniPatientService.get("partial_identifier", shouldMatchExactPatientId); | ||
verify(patientDao).getPatients("partial_identifier", shouldMatchExactPatientId); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we add this field here we also need to make sure it's respected by existing API methods that use PatientSearchParameters.
(It would be okay to skip the gender field when searching for similar patients, if your timeline to finish this is tight.)