Skip to content

Commit

Permalink
TRUNK-425 Fixed person attribute search
Browse files Browse the repository at this point in the history
  • Loading branch information
@rkorytkowski
rkorytkowski committed Jan 2, 2017
1 parent 3a8d717 commit 2b9b2b8
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 21 deletions.
10 changes: 2 additions & 8 deletions api/src/main/java/org/openmrs/PersonAttribute.java
View file
Expand Up @@ -12,20 +12,13 @@
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.lucene.analysis.core.LowerCaseFilterFactory;
import org.apache.lucene.analysis.standard.StandardFilterFactory;
import org.apache.lucene.analysis.standard.StandardTokenizerFactory;
import org.hibernate.search.annotations.Analyze;
import org.hibernate.search.annotations.Analyzer;
import org.hibernate.search.annotations.AnalyzerDef;
import org.hibernate.search.annotations.Boost;
import org.hibernate.search.annotations.DocumentId;
import org.hibernate.search.annotations.Fields;
import org.hibernate.search.annotations.Indexed;
import org.hibernate.search.annotations.Field;
import org.hibernate.search.annotations.IndexedEmbedded;
import org.hibernate.search.annotations.TokenFilterDef;
import org.hibernate.search.annotations.TokenizerDef;
import org.openmrs.api.context.Context;
import org.openmrs.api.db.hibernate.search.LuceneAnalyzers;
import org.openmrs.util.OpenmrsClassLoader;
Expand Down Expand Up @@ -60,7 +53,8 @@ public class PersonAttribute extends BaseOpenmrsData implements java.io.Serializ

@IndexedEmbedded(includeEmbeddedObjectId = true)
private Person person;


@IndexedEmbedded
private PersonAttributeType attributeType;

@Fields({
Expand Down
4 changes: 3 additions & 1 deletion api/src/main/java/org/openmrs/PersonAttributeType.java
View file
Expand Up @@ -12,6 +12,7 @@
import java.util.Comparator;

import org.codehaus.jackson.annotate.JsonIgnore;
import org.hibernate.search.annotations.Field;
import org.openmrs.util.OpenmrsUtil;

/**
Expand All @@ -28,7 +29,8 @@ public class PersonAttributeType extends BaseOpenmrsMetadata implements java.io.
private Integer foreignKey;

private Double sortWeight;


@Field
private Boolean searchable = false;

private Privilege editPrivilege;
Expand Down
7 changes: 5 additions & 2 deletions api/src/main/java/org/openmrs/api/db/hibernate/HibernatePatientDAO.java
View file
Expand Up @@ -711,8 +711,9 @@ public List<Patient> findPatients(String query, boolean includeVoided, Integer s
if (start == null) {
start = 0;
}
if (length == null) {
length = Integer.MAX_VALUE;
Integer maxLength = HibernatePersonDAO.getMaximumSearchResults();
if (length == null || length > maxLength) {
length = maxLength;
}
query = LuceneQuery.escapeQuery(query);

Expand Down Expand Up @@ -900,6 +901,8 @@ private LuceneQuery<PersonAttribute> getPersonAttributeLuceneQuery(String query,
luceneQuery.include("person.voided", false);
}

luceneQuery.include("attributeType.searchable", true);

if (skipSame != null) {
luceneQuery.skipSame("person.personId", skipSame);
} else {
Expand Down
18 changes: 15 additions & 3 deletions api/src/main/java/org/openmrs/api/impl/PersonServiceImpl.java
View file
Expand Up @@ -132,6 +132,7 @@ public void purgePersonAttributeType(PersonAttributeType type) throws APIExcepti
*/
public PersonAttributeType savePersonAttributeType(PersonAttributeType type) throws APIException {
checkIfPersonAttributeTypesAreLocked();

if (type.getSortWeight() == null) {
List<PersonAttributeType> allTypes = Context.getPersonService().getAllPersonAttributeTypes();
if (allTypes.size() > 0) {
Expand All @@ -140,8 +141,12 @@ public PersonAttributeType savePersonAttributeType(PersonAttributeType type) thr
type.setSortWeight(1.0);
}
}


boolean updateExisting = false;

if (type.getId() != null) {
updateExisting = true;

String oldTypeName = dao.getSavedPersonAttributeTypeName(type);
String newTypeName = type.getName();

Expand All @@ -165,8 +170,15 @@ public PersonAttributeType savePersonAttributeType(PersonAttributeType type) thr
}
}
}

return dao.savePersonAttributeType(type);

PersonAttributeType attributeType = dao.savePersonAttributeType(type);

if (updateExisting) {
//we need to update index in case searchable property has changed
Context.updateSearchIndexForType(PersonAttribute.class);
}

return attributeType;
}

/**
Expand Down
35 changes: 29 additions & 6 deletions api/src/test/java/org/openmrs/api/db/PatientDAOTest.java
View file
Expand Up @@ -10,7 +10,6 @@
package org.openmrs.api.db;

import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.RandomStringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hamcrest.FeatureMatcher;
Expand All @@ -24,11 +23,11 @@
import org.openmrs.Patient;
import org.openmrs.PatientIdentifier;
import org.openmrs.PatientIdentifierType;
import org.openmrs.Person;
import org.openmrs.PersonAttribute;
import org.openmrs.PersonAttributeType;
import org.openmrs.PersonName;
import org.openmrs.api.PatientService;
import org.openmrs.api.PersonService;
import org.openmrs.api.context.Context;
import org.openmrs.api.db.hibernate.HibernatePatientDAO;
import org.openmrs.api.db.hibernate.HibernatePersonDAO;
Expand All @@ -37,10 +36,9 @@
import org.openmrs.test.Verifies;
import org.openmrs.util.GlobalPropertiesTestHelper;
import org.openmrs.util.OpenmrsConstants;
import org.springframework.beans.factory.annotation.Autowired;

import java.io.File;
import java.io.FileInputStream;
import java.io.InputStream;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -50,10 +48,10 @@
import java.util.Set;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;

public class PatientDAOTest extends BaseContextSensitiveTest {

Expand All @@ -66,6 +64,9 @@ public class PatientDAOTest extends BaseContextSensitiveTest {
private PatientDAO dao = null;

private PatientService pService = null;

@Autowired
private PersonService personService;

private GlobalPropertiesTestHelper globalPropertiesTestHelper;

Expand Down Expand Up @@ -2164,9 +2165,27 @@ public void getDuplicatePatients_shouldGetDuplicatesWithBirthDateInvalidAttribut
Assert.assertEquals(31, patients.size());
}

@Test
public void getPatients_shouldFindOnlySearchablePersonAttributes() throws Exception {
PersonAttributeType attributeType = personService.getPersonAttributeTypeByName("Birthplace");
attributeType.setSearchable(false);
personService.savePersonAttributeType(attributeType);

List<Patient> patients = pService.getPatients("London");
assertThat(patients, is(empty()));

attributeType = personService.getPersonAttributeTypeByName("Birthplace");
attributeType.setSearchable(true);
personService.savePersonAttributeType(attributeType);

patients = pService.getPatients("London");
Patient patient = pService.getPatient(501);
assertThat(patients, contains(patient));
}

@Test
@Ignore("Designated for manual runs")
public void patientSearchPerformanceTest() throws Exception {
public void getPatients_shouldFindPatientsEfficiently() throws Exception {
URL givenNamesIn = getClass().getResource("/org/openmrs/api/db/givenNames.csv");
List<String> givenNames = FileUtils.readLines(new File(givenNamesIn.toURI()));
URL familyNamesIn = getClass().getResource("/org/openmrs/api/db/familyNames.csv");
Expand All @@ -2175,7 +2194,11 @@ public void patientSearchPerformanceTest() throws Exception {

PatientService patientService = Context.getPatientService();
PatientIdentifierType idType = patientService.getPatientIdentifierTypeByName("Old Identification Number");

PersonAttributeType attributeType = Context.getPersonService().getPersonAttributeTypeByName("Birthplace");
attributeType.setSearchable(true);
Context.getPersonService().savePersonAttributeType(attributeType);

Location location = Context.getLocationService().getLocation(1);
Random random = new Random(100); //set the seed to have repeatable results
List<String> generatedPatients = new ArrayList<>();
Expand Down
2 changes: 1 addition & 1 deletion api/src/test/resources/org/openmrs/include/standardTestDataset.xml
View file
Expand Up @@ -258,7 +258,7 @@
<person_attribute_type person_attribute_type_id="2" name="Birthplace" description="Location of persons birth" format="java.lang.String" searchable="false" creator="1" date_created="2007-05-04 09:59:23.0" retired="false" uuid="54fc8400-1683-4d71-a1ac-98d40836ff7c" sort_weight="3"/>
<person_attribute_type person_attribute_type_id="8" name="Civil Status" description="Marriage status of this person" format="org.openmrs.Concept" searchable="false" creator="1" date_created="2008-08-15 15:53:36.0" retired="false" uuid="a0f5521c-dbbd-4c10-81b2-1b7ab18330df" sort_weight="2"/>
<person_attribute person_attribute_id="1" person_id="501" value="NULL" person_attribute_type_id="1" creator="1" date_created="2008-08-15 15:46:47.0" voided="false" uuid="0768f3da-b692-44b7-a33f-abf2c450474e"/>
<person_attribute person_attribute_id="2" person_id="501" value="NULL" person_attribute_type_id="2" creator="1" date_created="2008-08-15 15:46:47.0" voided="false" uuid="56beb8bd-287c-47f2-9786-a7b98c933c04"/>
<person_attribute person_attribute_id="2" person_id="501" value="London" person_attribute_type_id="2" creator="1" date_created="2008-08-15 15:46:47.0" voided="false" uuid="56beb8bd-287c-47f2-9786-a7b98c933c04"/>
<person_attribute person_attribute_id="3" person_id="502" value="NULL" person_attribute_type_id="1" creator="1" date_created="2008-08-15 15:57:09.0" voided="false" uuid="898f6ecd-78f1-42a5-8768-e0c219a3fa7f"/>
<person_attribute person_attribute_id="4" person_id="502" value="NULL" person_attribute_type_id="2" creator="1" date_created="2008-08-15 15:57:09.0" voided="false" uuid="9b5e9fc6-eeb0-4b69-a699-e2f9f655cfec"/>
<person_attribute person_attribute_id="5" person_id="502" value="NULL" person_attribute_type_id="8" creator="1" date_created="2008-08-15 15:57:09.0" voided="false" uuid="463db590-b7a9-47e9-897a-c0506ba21ec9"/>
Expand Down

0 comments on commit 2b9b2b8

Please sign in to comment.