RESTWS-960: Consolidate Location Search Handling (#634)

Author: mogoodrich

omod-1.8/src/main/java/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/LocationResource1_8.java

@@ -9,6 +9,7 @@
  */
 package org.openmrs.module.webservices.rest.web.v1_0.resource.openmrs1_8;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
@@ -18,9 +19,9 @@
 import io.swagger.models.properties.RefProperty;
 import io.swagger.models.properties.StringProperty;
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.openmrs.Location;
 import org.openmrs.LocationTag;
-import org.openmrs.api.LocationService;
 import org.openmrs.api.context.Context;
 import org.openmrs.module.webservices.rest.web.RequestContext;
 import org.openmrs.module.webservices.rest.web.RestConstants;
@@ -35,6 +36,8 @@
 import org.openmrs.module.webservices.rest.web.resource.impl.NeedsPaging;
 import org.openmrs.module.webservices.rest.web.response.ResponseException;
 
+import static org.openmrs.util.PrivilegeConstants.VIEW_LOCATIONS;
+
 /**
  * {@link Resource} for {@link Location}, supporting standard CRUD operations
  */
@@ -249,39 +252,53 @@ public void purge(Location location, RequestContext context) throws ResponseExce
 	protected NeedsPaging<Location> doGetAll(RequestContext context) {
 		return new NeedsPaging<Location>(Context.getLocationService().getAllLocations(context.getIncludeAll()), context);
 	}
-	
+
 	/**
 	 * @see org.openmrs.module.webservices.rest.web.resource.impl.DelegatingCrudResource#doSearch(org.openmrs.module.webservices.rest.web.RequestContext)
-	 *      A query string and/or a tag uuid can be passed in; if both are passed in, returns an
+	 *      A query string and/or a tag (referenced by name or uuid) can be passed in; if both are passed in, returns an
 	 *      intersection of the results; excludes retired locations
 	 */
 	@Override
 	protected PageableResult doSearch(RequestContext context) {
-		
-		LocationService locationService = Context.getLocationService();
-		
-		String tagUuid = context.getParameter("tag");
+
+		String tag = context.getParameter("tag");
 		String query = context.getParameter("q");
-		
+
 		List<Location> locationsByTag = null;
 		List<Location> locationsByQuery = null;
-		
-		if (tagUuid != null) {
-			LocationTag locationTag = locationService.getLocationTagByUuid(tagUuid);
-			locationsByTag = locationService.getLocationsHavingAllTags(Arrays.asList(locationTag));
+
+		if (StringUtils.isNotBlank(tag)) {
+			locationsByTag = new ArrayList<Location>();
+
+			try {
+				Context.addProxyPrivilege(VIEW_LOCATIONS); //Not using PrivilegeConstants.VIEW_LOCATIONS which was removed in platform 1.11+
+				Context.addProxyPrivilege("Get Locations"); //1.11+
+
+				LocationTag locationTag = Context.getLocationService().getLocationTagByUuid(tag);
+				if (locationTag == null) {
+					locationTag = Context.getLocationService().getLocationTagByName(tag);
+				}
+
+				if (locationTag != null) {
+					locationsByTag = Context.getLocationService().getLocationsHavingAllTags(Arrays.asList(locationTag));
+				}
+			} finally {
+				Context.removeProxyPrivilege(VIEW_LOCATIONS); //Not using PrivilegeConstants.VIEW_LOCATIONS which was removed in platform 1.11+
+				Context.removeProxyPrivilege("Get Locations"); //1.11+
+			}
 		}
-		
-		if (query != null) {
-			locationsByQuery = locationService.getLocations(query);
+
+		if (StringUtils.isNotBlank(query)) {
+			locationsByQuery = Context.getLocationService().getLocations(query);
 		}
-		
+
 		if (locationsByTag == null) {
 			return new NeedsPaging<Location>(locationsByQuery, context);
 		} else if (locationsByQuery == null) {
 			return new NeedsPaging<Location>(locationsByTag, context);
 		} else {
 			return new NeedsPaging<Location>(
-			        (List<Location>) CollectionUtils.intersection(locationsByQuery, locationsByTag), context);
+					(List<Location>) CollectionUtils.intersection(locationsByQuery, locationsByTag), context);
 		}
 	}
 }

omod-1.8/src/main/java/org/openmrs/module/webservices/rest/web/v1_0/search/openmrs1_8/LocationSearchHandler.java

@@ -287,6 +287,20 @@ public void shouldSearchAndReturnListOfLocationsWithSpecifiedTag() throws Except
 		Assert.assertEquals(1, hits.size()); // should ignore retired location?
 		Assert.assertEquals(service.getLocation(1).getUuid(), PropertyUtils.getProperty(hits.get(0), "uuid"));
 	}
+
+	@Test
+	public void shouldSearchAndReturnListOfLocationsWithSpecifiedReferencedByName() throws Exception {
+
+		executeDataSet(LOCATION_TAG_INITIAL_XML);
+
+		MockHttpServletRequest req = request(RequestMethod.GET, getURI());
+		req.addParameter("tag", "General Hospital");
+
+		SimpleObject result = deserialize(handle(req));
+		List<Object> hits = (List<Object>) result.get("results");
+		Assert.assertEquals(1, hits.size()); // should ignore retired location?
+		Assert.assertEquals(service.getLocation(1).getUuid(), PropertyUtils.getProperty(hits.get(0), "uuid"));
+	}
 
 	@Test
 	public void shouldSearchAndReturnListOfLocationsWithSpecifiedTagAndQueryString() throws Exception {

omod-1.9/src/test/java/org/openmrs/module/webservices/rest/web/v1_0/controller/openmrs1_9/LocationController1_9Test.java

@@ -16,7 +16,6 @@
 import org.openmrs.module.webservices.rest.SimpleObject;
 import org.openmrs.module.webservices.rest.web.RestTestConstants1_8;
 import org.openmrs.module.webservices.rest.web.v1_0.controller.MainResourceControllerTest;
-import org.openmrs.module.webservices.rest.web.v1_0.search.openmrs1_8.LocationSearchHandler;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.web.bind.annotation.RequestMethod;
 
@@ -47,11 +46,7 @@ public long getAllCount() {
 	public String getUuid() {
 		return RestTestConstants1_8.PATIENT_UUID;
 	}
-	
-	/**
-	 * @verifies return location by tag uuid
-	 * @see LocationSearchHandler#getSearchConfig()
-	 */
+
 	@Test
 	public void getSearchConfig_shouldReturnPatientByIdentifier() throws Exception {
 		MockHttpServletRequest req = request(RequestMethod.GET, getURI());