BAH-460

- Refactor PatientDaoImpl#getSimilarPatientsUsingLuceneSearch(...) - Sending only relevant parameters - Extracted methods for cleaner code - Improvement: Instead of hitting database for Patients lookup, we construct a Patient out of a Person - Added unit test to check limit parameter - BahmniPatientServiceImpl#searchSimilarPatients(...) - Changes in calling to PatientDao - Added unit testt
Bahmni · May 15, 2018 · ee21dca · ee21dca
1 parent fc8d8f5
commit ee21dca
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 75 deletions.
diff --git a/bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/dao/PatientDao.java b/bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/dao/PatientDao.java
@@ -3,7 +3,6 @@
 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;
 
@@ -20,11 +19,7 @@ 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);
+ List<PatientResponse> getSimilarPatientsUsingLuceneSearch(String name, String gender, String loginLocationUuid, Integer length);
 
  public Patient getPatient(String identifier);
 

diff --git a/bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/dao/impl/PatientDaoImpl.java b/bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/dao/impl/PatientDaoImpl.java
@@ -32,15 +32,9 @@
 import org.openmrs.module.bahmniemrapi.visitlocation.BahmniVisitLocationServiceImpl;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Repository;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
-
-import static java.util.stream.Collectors.reducing;
+
+import java.util.*;
+
 import static java.util.stream.Collectors.toList;
 
 @Repository
@@ -93,47 +87,39 @@ public List<PatientResponse> getPatientsUsingLuceneSearch(String identifier, Str
  List<PatientResponse> patientResponses = patientIdentifiers.stream()
  .map(patientIdentifier -> {
  Patient patient = patientIdentifier.getPatient();
- 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;
+ return toPatientResponse(patientResponseMapper, patient, loginLocationUuid, addressSearchResultFields, patientSearchResultFields, programAttributes, uniquePatientIds);
  }).filter(Objects::nonNull)
  .collect(toList());
  return patientResponses;
  }
 
+ // TODO BAH-460 create a class for the search fields
  @Override
- public List<PatientResponse> getSimilarPatientsUsingLuceneSearch(String identifier, 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) {
-
- validateSearchParams(customAttributeFields, programAttributeFieldName, addressFieldName);
+ public List<PatientResponse> getSimilarPatientsUsingLuceneSearch(String name, String gender, String loginLocationUuid, Integer length) {
  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<>();
+ // TODO BAH-460 Maybe we can remove the new HashMap<>() from the call. It used to be response from validateSearchParams(...)
  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)
+ .map(patient -> toPatientResponse(patientResponseMapper, patient, loginLocationUuid, new HashMap<>(), uniquePatientIds)).filter(Objects::nonNull)
  .collect(toList());
  return patientResponses;
  }
 
+ private PatientResponse toPatientResponse(PatientResponseMapper patientResponseMapper, Patient patient, String loginLocationUuid, Map<Object, Object> programAttributes, Set<Integer> uniquePatientIds) {
+ return toPatientResponse(patientResponseMapper, patient, loginLocationUuid, null, null, programAttributes, uniquePatientIds);
+ }
+
+ private PatientResponse toPatientResponse(PatientResponseMapper patientResponseMapper, Patient patient, String loginLocationUuid, String[] addressSearchResultFields, String[] patientSearchResultFields, Map<Object, Object> programAttributes, Set<Integer> uniquePatientIds) {
+ 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;
+ }
+ }
 
  private List<Patient> getPatientsByNameAndGender(String name, String gender, Integer length) {
  HibernatePatientDAO patientDAO = new HibernatePatientDAO();
@@ -153,10 +139,11 @@ private List<Patient> getPatientsByNameAndGender(String name, String gender, Int
  && 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())));
+ persons.forEach(person -> patients.add(new Patient(person.getPerson())));
  return patients;
  }
 
+
  private Boolean checkGender(Person person, String gender) {
  if(gender != null && !gender.isEmpty()){
  return gender.equals(person.getGender());

diff --git a/...api/src/main/java/org/bahmni/module/bahmnicore/service/impl/BahmniPatientServiceImpl.java b/...api/src/main/java/org/bahmni/module/bahmnicore/service/impl/BahmniPatientServiceImpl.java
@@ -1,31 +1,26 @@
 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
 public class BahmniPatientServiceImpl implements BahmniPatientService {
+ private static final int SIMILAR_PATIENT_RESULT_LENGTH = 5;
  private PersonService personService;
  private ConceptService conceptService;
  private PatientDao patientDao;
@@ -91,21 +86,10 @@ public List<PatientResponse> luceneSearch(PatientSearchParameters searchParamete
 
  @Override
  public List<PatientResponse> searchSimilarPatients(PatientSearchParameters searchParameters) {
- return patientDao.getSimilarPatientsUsingLuceneSearch(searchParameters.getIdentifier(),
- searchParameters.getName(),
+ return patientDao.getSimilarPatientsUsingLuceneSearch(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());
+ SIMILAR_PATIENT_RESULT_LENGTH);
  }
 
  @Override

diff --git a/...api/src/test/java/org/bahmni/module/bahmnicore/dao/impl/BahmniPatientDaoImplLuceneIT.java b/...api/src/test/java/org/bahmni/module/bahmnicore/dao/impl/BahmniPatientDaoImplLuceneIT.java
@@ -7,7 +7,6 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-import org.openmrs.Patient;
 import org.springframework.beans.factory.annotation.Autowired;
 
 import java.util.List;
@@ -215,11 +214,10 @@ public void shouldNotReturnDuplicatePatientsEvenIfTwoIdentifiersMatches() {
 
  @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);
+ List<PatientResponse> patients = patientDao.getSimilarPatientsUsingLuceneSearch("Peet", "", "c36006e5-9fbb-4f20-866b-0ece245615a1", 5);
  PatientResponse patient1 = patients.get(0);
  PatientResponse patient2 = patients.get(1);
- 
+
  assertEquals(2, patients.size());
  assertEquals(patient1.getGivenName(), "Horatio");
  assertEquals(patient1.getMiddleName(), "Peeter");
@@ -229,19 +227,28 @@ public void shouldSearchSimilarPatientByPatientName() {
  assertEquals(patient2.getFamilyName(), "Sinha");
  }
 
+ @Test
+ public void shouldSearchSimilarPatientByPatientNameAndUseLimitResult() {
+ List<PatientResponse> patients = patientDao.getSimilarPatientsUsingLuceneSearch("Peet", "", "c36006e5-9fbb-4f20-866b-0ece245615a1", 1);
+ assertEquals("Should limit number of results",1, patients.size());
+ PatientResponse patient1 = patients.get(0);
+
+ assertEquals(patient1.getGivenName(), "Horatio");
+ assertEquals(patient1.getMiddleName(), "Peeter");
+ assertEquals(patient1.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);
+ List<PatientResponse> patients = patientDao.getSimilarPatientsUsingLuceneSearch("Peet", "F", "c36006e5-9fbb-4f20-866b-0ece245615a1", 5);
  PatientResponse patient1 = patients.get(0);
 
- for(PatientResponse response: patients) {
- System.out.println(response.getGivenName() + " " + response.getMiddleName() + " " + response.getFamilyName());
- }
  assertEquals(1, patients.size());
  assertEquals(patient1.getGivenName(), "John");
  assertEquals(patient1.getMiddleName(), "Peeter");
  assertEquals(patient1.getFamilyName(), "Sinha");
  }
 
+ //TODO missing tests: by limit, parameters verification
+
 }
diff --git a/...src/test/java/org/bahmni/module/bahmnicore/service/impl/BahmniPatientServiceImplTest.java b/...src/test/java/org/bahmni/module/bahmnicore/service/impl/BahmniPatientServiceImplTest.java
@@ -1,7 +1,6 @@
 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.dao.PatientDao;
 import org.junit.Before;
@@ -11,14 +10,15 @@
 import org.openmrs.PersonAttributeType;
 import org.openmrs.api.ConceptService;
 import org.openmrs.api.PersonService;
+import org.openmrs.module.webservices.rest.web.RequestContext;
 
+import javax.servlet.http.HttpServletRequest;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 
 import static junit.framework.Assert.assertEquals;
-import static org.mockito.Mockito.anyInt;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 import static org.mockito.MockitoAnnotations.initMocks;
 
 public class BahmniPatientServiceImplTest {
@@ -27,6 +27,8 @@ public class BahmniPatientServiceImplTest {
  @Mock
  private ConceptService conceptService;
  @Mock
+ RequestContext requestContext;
+ @Mock
  private PatientDao patientDao;
 
  private BahmniPatientServiceImpl bahmniPatientService;
@@ -70,4 +72,22 @@ public void shouldGetPatientByPartialIdentifier() throws Exception {
  verify(patientDao).getPatients("partial_identifier", shouldMatchExactPatientId);
  }
 
+ // TODO BAH-460 Add test that verifies call to PatioenDaoImpl for searching similar patients
+
+
+ @Test
+ public void shouldCallgetSimilarPatientsUsingLuceneSearch() {
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(requestContext.getRequest()).thenReturn(request);
+ when(request.getParameterMap()).thenReturn(new HashMap<>());
+
+ PatientSearchParameters patientSearchParameters = new PatientSearchParameters(requestContext);
+ patientSearchParameters.setName("John");
+ patientSearchParameters.setGender("M");
+ patientSearchParameters.setLoginLocationUuid("someUUid");
+
+
+ bahmniPatientService.searchSimilarPatients(patientSearchParameters);
+ verify(patientDao).getSimilarPatientsUsingLuceneSearch("John", "M", "someUUid", 5);
+ }
 }