From f1b007c09463db121c7cb09b9a34fada40c1bc0f Mon Sep 17 00:00:00 2001 From: Mark Goodrich Date: Tue, 1 Jun 2021 12:43:29 -0400 Subject: [PATCH] TRUNK-6007 (#3792) * TRUNK-6007: When save person attribute type, only update search index if "searchable" property is saved --- .../java/org/openmrs/api/db/PersonDAO.java | 19 +++++++++++---- .../api/db/hibernate/HibernatePersonDAO.java | 22 ++++++++++++------ .../openmrs/api/impl/PersonServiceImpl.java | 23 +++++++++++-------- .../org/openmrs/api/db/PersonDAOTest.java | 17 ++++++++++---- 4 files changed, 56 insertions(+), 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 index b420c6a3630d..5aa62385f0a5 100644 --- 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 *

@@ -199,6 +199,17 @@ public List 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 index b22fe643a768..c40e91563014 100644 --- 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.
*
@@ -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 index 653855ee8931..2a5138badae6 100644 --- 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 *

@@ -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 index 640c159d561b..9193ac9986aa 100644 --- a/api/src/test/java/org/openmrs/api/db/PersonDAOTest.java +++ b/api/src/test/java/org/openmrs/api/db/PersonDAOTest.java @@ -9,10 +9,6 @@ */ 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; @@ -20,6 +16,10 @@ 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); + } + }