TRUNK-6007 (openmrs#3792)

* TRUNK-6007: When save person attribute type, only update search index if "searchable" property is saved
Parth59 · Jun 1, 2021 · f1b007c · f1b007c
1 parent 501662b
commit f1b007c
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 25 deletions.
diff --git a/api/src/main/java/org/openmrs/api/db/PersonDAO.java b/api/src/main/java/org/openmrs/api/db/PersonDAO.java
@@ -9,10 +9,6 @@
  */
 package org.openmrs.api.db;
 
-import java.util.Date;
-import java.util.List;
-import java.util.Set;
-
 import org.openmrs.Person;
 import org.openmrs.PersonAddress;
 import org.openmrs.PersonAttribute;
@@ -23,6 +19,10 @@
 import org.openmrs.person.PersonMergeLog;
 import org.openmrs.util.OpenmrsConstants;
 
+import java.util.Date;
+import java.util.List;
+import java.util.Set;
+
 /**
  * Person-related database functions
  * <p>
@@ -199,6 +199,17 @@ public List<Relationship> getRelationships(Person fromPerson, Person toPerson, R
 	 * @should get saved personAttributeType name from database
 	 */
 	public String getSavedPersonAttributeTypeName(PersonAttributeType personAttributeType);
+
+	/**
+	 * Gets the value of the searchable property currently saved in the database for the given personAttributeType,
+	 * bypassing any caches. This is used when saving an personAttributeType, so that we can
+	 * determine if the searchable property has changed, which we use to determine whether we need to update the 
+	 * Lucene index
+	 * 
+	 * @param personAttributeType
+	 * @return the searchable property currently in the database for this personAttributeType
+	 */
+	public Boolean getSavedPersonAttributeTypeSearchable(PersonAttributeType personAttributeType);
 
 	/**
 	 * @see org.openmrs.api.PersonService#getAllRelationshipTypes(boolean)

diff --git a/api/src/main/java/org/openmrs/api/db/hibernate/HibernatePersonDAO.java b/api/src/main/java/org/openmrs/api/db/hibernate/HibernatePersonDAO.java
@@ -9,12 +9,6 @@
  */
 package org.openmrs.api.db.hibernate;
 
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Set;
-
 import org.apache.commons.lang3.StringUtils;
 import org.hibernate.Criteria;
 import org.hibernate.Query;
@@ -40,6 +34,12 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
 /**
  * Hibernate specific Person database methods. <br>
  * <br>
@@ -636,7 +636,15 @@ public String getSavedPersonAttributeTypeName(PersonAttributeType personAttribut
 		sql.setInteger("personAttributeTypeId", personAttributeType.getId());
 		return (String) sql.uniqueResult();
 	}
-
+
+	@Override
+	public Boolean getSavedPersonAttributeTypeSearchable(PersonAttributeType personAttributeType) {
+		SQLQuery sql = sessionFactory.getCurrentSession().createSQLQuery(
+			"select searchable from person_attribute_type where person_attribute_type_id = :personAttributeTypeId");
+		sql.setInteger("personAttributeTypeId", personAttributeType.getId());
+		return (Boolean) sql.uniqueResult();
+	}
+
 	/**
 	 * @see org.openmrs.api.db.PersonDAO#getPersonByUuid(java.lang.String)
 	 */

diff --git a/api/src/main/java/org/openmrs/api/impl/PersonServiceImpl.java b/api/src/main/java/org/openmrs/api/impl/PersonServiceImpl.java
@@ -9,13 +9,6 @@
  */
 package org.openmrs.api.impl;
 
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 import org.apache.commons.lang3.StringUtils;
 import org.openmrs.GlobalProperty;
 import org.openmrs.Person;
@@ -43,6 +36,13 @@
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.util.Assert;
 
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 /**
  * Default implementation of the PersonService
  * <p>
@@ -182,9 +182,12 @@ public PersonAttributeType savePersonAttributeType(PersonAttributeType type) thr
 
 		PersonAttributeType attributeType = dao.savePersonAttributeType(type);
 
-		if (updateExisting) {
-			//we need to update index in case searchable property has changed
-			Context.updateSearchIndexForType(PersonAttribute.class);
+		if (updateExisting ) {
+			Boolean oldSearchable = dao.getSavedPersonAttributeTypeSearchable(type);
+			if (oldSearchable == null || !oldSearchable.equals(type.getSearchable())) {
+				//we need to update index searchable property has changed
+				Context.updateSearchIndexForType(PersonAttribute.class);
+			}
 		}
 
 		return attributeType;

diff --git a/api/src/test/java/org/openmrs/api/db/PersonDAOTest.java b/api/src/test/java/org/openmrs/api/db/PersonDAOTest.java
@@ -9,17 +9,17 @@
  */
 package org.openmrs.api.db;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.openmrs.PersonAttributeType;
 import org.openmrs.PersonName;
 import org.openmrs.api.context.Context;
 import org.openmrs.test.BaseContextSensitiveTest;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+
 public class PersonDAOTest extends BaseContextSensitiveTest {
 
 	private PersonDAO dao = null;
@@ -71,4 +71,13 @@ public void getPersonName_shouldNotGetPersonNameGivenInvalidId() {
 		assertNull(personName);
 	}
 
+	@Test
+	public void getSavedPersonAttributeTypeSearchable_shouldFetchSearchablePropertyForAPersonAttributeTypeBypassingCache(){
+		PersonAttributeType pat = dao.getPersonAttributeType(1);
+		pat.setSearchable(true);
+		// should still be false in the DB
+		Boolean searchable = dao.getSavedPersonAttributeTypeSearchable(pat);
+		assertFalse(searchable);
+	}
+
 }