O3-4544: Improve FHIR Location endpoint performance openmrs#559

Author: pwargulak

api-2.6/src/test/java/org/openmrs/module/fhir2/api/impl/FhirMedicationDispenseServiceImpl_2_6Test.java

@@ -18,6 +18,7 @@
 import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyCollection;
 import static org.mockito.Mockito.when;
 
 import java.util.Collections;
@@ -238,6 +239,7 @@ public void searchMedicationDispenses_shouldGetSearchResults() {
 		    new SearchQueryBundleProvider<>(theParams, dao, translator, globalPropertyService, searchQueryInclude));
 		when(searchQueryInclude.getIncludedResources(any(), any())).thenReturn(Collections.emptySet());
 		when(translator.toFhirResource(openmrsDispense)).thenReturn(fhirDispense);
+		when(translator.toFhirResources(anyCollection())).thenCallRealMethod();
 
 		MedicationDispenseSearchParams params = new MedicationDispenseSearchParams();
 		params.setId(idParam);

api/src/main/java/org/openmrs/module/fhir2/api/dao/FhirLocationDao.java

@@ -11,7 +11,9 @@
 
 import javax.annotation.Nonnull;
 
+import java.util.Collection;
 import java.util.List;
+import java.util.Map;
 
 import org.openmrs.Location;
 import org.openmrs.LocationAttribute;
@@ -31,6 +33,9 @@ public interface FhirLocationDao extends FhirDao<Location> {
 	List<LocationAttribute> getActiveAttributesByLocationAndAttributeTypeUuid(@Nonnull Location location,
 	        @Nonnull String locationAttributeTypeUuid);
 
+	Map<Location, List<LocationAttribute>> getActiveAttributesByLocationsAndAttributeTypeUuid(
+	        @Nonnull Collection<Location> location, @Nonnull String locationAttributeTypeUuid);
+	
 	@Override
 	@Authorized(PrivilegeConstants.MANAGE_LOCATIONS)
 	Location createOrUpdate(@Nonnull Location newEntry);

api/src/main/java/org/openmrs/module/fhir2/api/dao/impl/BaseFhirDao.java

@@ -33,6 +33,7 @@
 import lombok.AccessLevel;
 import lombok.Getter;
 import lombok.Setter;
+import lombok.extern.slf4j.Slf4j;
 import org.hibernate.CacheMode;
 import org.hibernate.Criteria;
 import org.hibernate.Hibernate;
@@ -68,6 +69,7 @@
  * @param <T> the {@link OpenmrsObject} managed by this Dao
  */
 @Transactional
+@Slf4j
 public abstract class BaseFhirDao<T extends OpenmrsObject & Auditable> extends BaseDao implements FhirDao<T> {
 
 	@SuppressWarnings("UnstableApiUsage")
@@ -249,6 +251,7 @@ public List<T> getSearchResults(@Nonnull SearchParameterMap theParams) {
 
 			results = idsCriteria.list();
 		}
+		
 		return results.stream().map(this::deproxyResult).collect(Collectors.toList());
 	}
 

api/src/main/java/org/openmrs/module/fhir2/api/dao/impl/FhirLocationDaoImpl.java

@@ -9,13 +9,21 @@
  */
 package org.openmrs.module.fhir2.api.dao.impl;
 
+import static java.util.stream.Collectors.groupingBy;
+import static java.util.stream.Collectors.toList;
 import static org.hibernate.criterion.Restrictions.eq;
 import static org.hibernate.criterion.Restrictions.or;
 
 import javax.annotation.Nonnull;
+import javax.persistence.criteria.CriteriaBuilder;
+import javax.persistence.criteria.CriteriaQuery;
+import javax.persistence.criteria.Predicate;
+import javax.persistence.criteria.Root;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 
 import ca.uhn.fhir.rest.param.ReferenceAndListParam;
@@ -93,6 +101,25 @@ public List<LocationAttribute> getActiveAttributesByLocationAndAttributeTypeUuid
 		        .list();
 	}
 
+	@Override
+	public Map<Location, List<LocationAttribute>> getActiveAttributesByLocationsAndAttributeTypeUuid(
+	        @Nonnull Collection<Location> location, @Nonnull String locationAttributeTypeUuid) {
+		final CriteriaBuilder criteriaBuilder = getSessionFactory().getCurrentSession().getCriteriaBuilder();
+		final CriteriaQuery<LocationAttribute> criteria = criteriaBuilder.createQuery(LocationAttribute.class);
+		final Root<LocationAttribute> locationAttributeRoot = criteria.from(LocationAttribute.class);
+		
+		final Predicate byId = locationAttributeRoot.get("location").get("locationId")
+		        .in(location.stream().map(Location::getLocationId).collect(toList()));
+		final Predicate byAttributeType = criteriaBuilder.equal(locationAttributeRoot.get("attributeType").get("uuid"),
+		    locationAttributeTypeUuid);
+		final Predicate byVoided = criteriaBuilder.equal(locationAttributeRoot.get("voided"), false);
+		
+		criteria.where(byId, byAttributeType, byVoided);
+		
+		return getSessionFactory().getCurrentSession().createQuery(criteria).getResultList().stream()
+		        .collect(groupingBy(LocationAttribute::getLocation));
+	}
+	
 	private void handleName(Criteria criteria, StringAndListParam namePattern) {
 		handleAndListParam(namePattern, (name) -> propertyLike("name", name)).ifPresent(criteria::add);
 	}

api/src/main/java/org/openmrs/module/fhir2/api/search/SearchQuery.java

@@ -18,7 +18,7 @@
 import org.openmrs.module.fhir2.api.FhirGlobalPropertyService;
 import org.openmrs.module.fhir2.api.dao.FhirDao;
 import org.openmrs.module.fhir2.api.search.param.SearchParameterMap;
-import org.openmrs.module.fhir2.api.translators.ToFhirTranslator;
+import org.openmrs.module.fhir2.api.translators.OpenmrsFhirTranslator;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
@@ -29,7 +29,7 @@
  * @param <T> FHIR generic translator Class
  */
 @Component
-public class SearchQuery<T extends OpenmrsObject & Auditable, U extends IBaseResource, O extends FhirDao<T>, V extends ToFhirTranslator<T, U>, W extends SearchQueryInclude<U>> {
+public class SearchQuery<T extends OpenmrsObject & Auditable, U extends IBaseResource, O extends FhirDao<T>, V extends OpenmrsFhirTranslator<T, U>, W extends SearchQueryInclude<U>> {
 
 	@Autowired
 	private FhirGlobalPropertyService globalPropertyService;

api/src/main/java/org/openmrs/module/fhir2/api/search/SearchQueryBundleProvider.java

@@ -16,13 +16,12 @@
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
-import java.util.Objects;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import ca.uhn.fhir.model.primitive.InstantDt;
 import ca.uhn.fhir.rest.api.server.IBundleProvider;
 import lombok.Getter;
+import lombok.extern.slf4j.Slf4j;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
 import org.openmrs.Auditable;
@@ -31,10 +30,11 @@
 import org.openmrs.module.fhir2.api.FhirGlobalPropertyService;
 import org.openmrs.module.fhir2.api.dao.FhirDao;
 import org.openmrs.module.fhir2.api.search.param.SearchParameterMap;
-import org.openmrs.module.fhir2.api.translators.ToFhirTranslator;
+import org.openmrs.module.fhir2.api.translators.OpenmrsFhirTranslator;
 import org.openmrs.module.fhir2.api.util.FhirUtils;
 import org.springframework.transaction.annotation.Transactional;
 
+@Slf4j
 public class SearchQueryBundleProvider<T extends OpenmrsObject & Auditable, U extends IBaseResource> implements IBundleProvider, Serializable {
 
 	private static final long serialVersionUID = 4L;
@@ -46,7 +46,7 @@ public class SearchQueryBundleProvider<T extends OpenmrsObject & Auditable, U ex
 
 	private final SearchParameterMap searchParameterMap;
 
-	private final ToFhirTranslator<T, U> translator;
+	private final OpenmrsFhirTranslator<T, U> translator;
 
 	@Getter
 	private final String uuid;
@@ -60,7 +60,7 @@ public class SearchQueryBundleProvider<T extends OpenmrsObject & Auditable, U ex
 	private final SearchQueryInclude<U> searchQueryInclude;
 
 	public SearchQueryBundleProvider(SearchParameterMap searchParameterMap, FhirDao<T> dao,
-	    ToFhirTranslator<T, U> translator, FhirGlobalPropertyService globalPropertyService,
+	    OpenmrsFhirTranslator<T, U> translator, FhirGlobalPropertyService globalPropertyService,
 	    SearchQueryInclude<U> searchQueryInclude) {
 		this.dao = dao;
 		this.published = InstantDt.withCurrentTime();
@@ -78,13 +78,12 @@ public List<IBaseResource> getResources(int fromIndex, int toIndex) {
 		searchParameterMap.setFromIndex(fromIndex);
 		searchParameterMap.setToIndex(toIndex);
 
-		List<U> returnedResourceList = dao.getSearchResults(searchParameterMap).stream().map(translator::toFhirResource)
-		        .filter(Objects::nonNull).collect(Collectors.toList());
+		List<U> resources = translator.toFhirResources(dao.getSearchResults(searchParameterMap));
 
-		Set<IBaseResource> includedResources = searchQueryInclude.getIncludedResources(returnedResourceList,
-		    this.searchParameterMap);
+		Set<IBaseResource> includedResources = searchQueryInclude.getIncludedResources(resources, this.searchParameterMap);
 
-		List<IBaseResource> resultList = new ArrayList<>(returnedResourceList);
+		List<IBaseResource> resultList = new ArrayList<>(resources.size() + includedResources.size());
+		resultList.addAll(resources);
 		resultList.addAll(includedResources);
 
 		return resultList;

api/src/main/java/org/openmrs/module/fhir2/api/translators/LocationTranslator.java

@@ -11,6 +11,9 @@
 
 import javax.annotation.Nonnull;
 
+import java.util.Collection;
+import java.util.List;
+
 import org.hl7.fhir.r4.model.Location;
 
 public interface LocationTranslator extends OpenmrsFhirUpdatableTranslator<org.openmrs.Location, Location> {
@@ -24,6 +27,15 @@ public interface LocationTranslator extends OpenmrsFhirUpdatableTranslator<org.o
 	@Override
 	Location toFhirResource(@Nonnull org.openmrs.Location openmrsLocation);
 
+	/**
+	 * Maps a collection of {@link org.openmrs.Location}s to a {@link org.hl7.fhir.r4.model.Location}
+	 *
+	 * @param openmrsLocations the collection of locations to translate
+	 * @return the mapping of OpenMRS location to corresponding FHIR location resource
+	 */
+	@Override
+	List<Location> toFhirResources(Collection<org.openmrs.Location> openmrsLocations);
+	
 	/**
 	 * Maps a {@link org.hl7.fhir.r4.model.Location} to an {@link org.openmrs.Location}
 	 * 

api/src/main/java/org/openmrs/module/fhir2/api/translators/OpenmrsFhirTranslator.java

@@ -9,10 +9,25 @@
  */
 package org.openmrs.module.fhir2.api.translators;
 
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+
 /**
  * Generic interface for a translator between OpenMRS data and FHIR resources
  * 
  * @param <T> OpenMRS data type
  * @param <U> FHIR resource type
  */
-public interface OpenmrsFhirTranslator<T, U> extends ToFhirTranslator<T, U>, ToOpenmrsTranslator<T, U> {}
+public interface OpenmrsFhirTranslator<T, U> extends ToFhirTranslator<T, U>, ToOpenmrsTranslator<T, U> {
+	
+	/**
+	 * Maps OpenMRS data elements to FHIR resources.
+	 *
+	 * @param data the collection of OpenMRS data elements to translate
+	 * @return the mapping of OpenMRS data element to corresponding FHIR resource
+	 */
+	default List<U> toFhirResources(Collection<T> data) {
+		return data.stream().distinct().map(this::toFhirResource).collect(Collectors.toList());
+	}
+}

api/src/main/java/org/openmrs/module/fhir2/api/translators/ToFhirTranslator.java

@@ -26,4 +26,5 @@ public interface ToFhirTranslator<T, U> extends FhirTranslator {
 	 * @return the corresponding FHIR resource
 	 */
 	U toFhirResource(@Nonnull T data);
+	
 }