Skip to content
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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public class PatientSearchParameters {
private Boolean filterPatientsByLocation;
private String identifier;
private String name;
private String gender;
private String addressFieldName;
private String addressFieldValue;
private Integer start;
Expand Down Expand Up @@ -43,6 +44,7 @@ public PatientSearchParameters(RequestContext context) {
} else {
this.setAddressFieldName("city_village");
}
this.setGender(context.getParameter("gender"));
this.setAddressFieldValue(context.getParameter("addressFieldValue"));
Map parameterMap = context.getRequest().getParameterMap();
this.setAddressSearchResultFields((String[]) parameterMap.get("addressSearchResultsConfig"));
Expand Down Expand Up @@ -71,6 +73,14 @@ public void setName(String name) {
this.name = name;
}

public String getGender() {

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.)

return gender;
}

public void setGender(String gender) {
this.gender = gender;
}

public String getAddressFieldName() {
return addressFieldName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ public PatientResponse map(Patient patient, String loginLocationUuid, String[] s
patientResponse.setFamilyName(patient.getFamilyName());
patientResponse.setGender(patient.getGender());
PatientIdentifier primaryIdentifier = patient.getPatientIdentifier();
patientResponse.setIdentifier(primaryIdentifier.getIdentifier());
if(primaryIdentifier != null) {
patientResponse.setIdentifier(primaryIdentifier.getIdentifier());
}
patientResponse.setPatientProgramAttributeValue(programAttributeValue);

mapExtraIdentifiers(patient, primaryIdentifier);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.bahmni.module.bahmnicore.contract.patient.response.PatientResponse;
import org.openmrs.Patient;
import org.openmrs.RelationshipType;
import org.openmrs.module.emrapi.patient.PatientProfile;

import java.util.List;

Expand All @@ -19,6 +20,12 @@ List<PatientResponse> getPatientsUsingLuceneSearch(String identifier, String nam
String programAttributeFieldName, String[] addressSearchResultFields,
String[] patientSearchResultFields, String loginLocationUuid, Boolean filterPatientsByLocation, Boolean filterOnAllIdentifiers);

List<PatientResponse> getSimilarPatientsUsingLuceneSearch(String identifer, String name, String gender, String customAttribute,
String addressFieldName, String addressFieldValue, Integer length,
Integer offset, String[] customAttributeFields, String programAttributeFieldValue,
String programAttributeFieldName, String[] addressSearchResultFields,
String[] patientSearchResultFields, String loginLocationUuid, Boolean filterPatientsByLocation, Boolean filterOnAllIdentifiers);

public Patient getPatient(String identifier);

public List<Patient> getPatients(String partialIdentifier, boolean shouldMatchExactPatientId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,

Choose a reason for hiding this comment

The 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.

Choose a reason for hiding this comment

The 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())));
Copy link

Choose a reason for hiding this comment

The 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 personLuceneQuery.getPatientNameQueryWithOrParser, we can access the Person via personName.getPerson() but here are accessing the database to get Patients. Do you have a hint for us on how to get Patients without hitting the DB? Can we simply cast them?

cc @eyalgo (who is picking up on the work done by Jian with Martina)

Choose a reason for hiding this comment

The 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();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package org.bahmni.module.bahmnicore.service;

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.openmrs.Patient;
import org.openmrs.Person;
import org.openmrs.RelationshipType;

import java.util.List;
Expand All @@ -15,6 +17,8 @@ public interface BahmniPatientService {

List<PatientResponse> luceneSearch(PatientSearchParameters searchParameters);

List<PatientResponse> searchSimilarPatients(PatientSearchParameters searchParameters);

public List<Patient> get(String partialIdentifier, boolean shouldMatchExactPatientId);

public List<RelationshipType> getByAIsToB(String aIsToB);
Expand Down
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
Expand Down Expand Up @@ -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(),

Choose a reason for hiding this comment

The 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Choose a reason for hiding this comment

The 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;

Choose a reason for hiding this comment

The 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;
Expand Down Expand Up @@ -67,4 +69,5 @@ public void shouldGetPatientByPartialIdentifier() throws Exception {
bahmniPatientService.get("partial_identifier", shouldMatchExactPatientId);
verify(patientDao).getPatients("partial_identifier", shouldMatchExactPatientId);
}

}
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
package org.bahmni.module.bahmnicore.web.v1_0.controller.search;

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.PatientResponse;
import org.bahmni.module.bahmnicore.service.BahmniPatientService;
import org.openmrs.Patient;
import org.openmrs.PatientIdentifier;
import org.openmrs.Person;
import org.openmrs.api.context.Context;
import org.openmrs.module.bahmniemrapi.visitlocation.BahmniVisitLocationServiceImpl;
import org.openmrs.module.webservices.rest.web.RequestContext;
import org.openmrs.module.webservices.rest.web.RestConstants;
import org.openmrs.module.webservices.rest.web.RestUtil;
Expand All @@ -15,6 +21,7 @@
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.ResponseBody;

import javax.servlet.http.HttpServletRequest;
Expand Down Expand Up @@ -66,4 +73,19 @@ public ResponseEntity<AlreadyPaged<PatientResponse>> luceneSearch(HttpServletReq
return new ResponseEntity(RestUtil.wrapErrorResponse(e, e.getMessage()), HttpStatus.BAD_REQUEST);
}
}

@RequestMapping(value="similar", method = RequestMethod.GET)
@ResponseBody
public ResponseEntity<AlreadyPaged<PatientResponse>> searchSimilarPerson(HttpServletRequest request,
HttpServletResponse response) throws ResponseException{
RequestContext requestContext = RestUtil.getRequestContext(request, response);
PatientSearchParameters searchParameters = new PatientSearchParameters(requestContext);
try {
List<PatientResponse> patients = bahmniPatientService.searchSimilarPatients(searchParameters);
AlreadyPaged alreadyPaged = new AlreadyPaged(requestContext, patients, false);
return new ResponseEntity(alreadyPaged, HttpStatus.OK);
}catch (IllegalArgumentException e){
return new ResponseEntity(RestUtil.wrapErrorResponse(e, e.getMessage()), HttpStatus.BAD_REQUEST);
}
}
}