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 17, 2018 · 6d1d72a · 6d1d72a
1 parent fc8d8f5
commit 6d1d72a
Show file tree

Hide file tree

Showing 5 changed files with 64 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,38 @@ 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;
     }
 
     @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 +138,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,19 @@ public void shouldGetPatientByPartialIdentifier() throws Exception {
         verify(patientDao).getPatients("partial_identifier", shouldMatchExactPatientId);
     }
 
+    @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);
+    }
 }